Page Menu
Home
Phorge
Search
Configure Global Search
Log In
Files
F3987752
D25823.1746585813.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Award Token
Flag For Later
Advanced/Developer...
View Handle
View Hovercard
Size
10 KB
Referenced Files
None
Subscribers
None
D25823.1746585813.diff
View Options
diff --git a/src/browse/query/ArcanistBrowsePathURIHardpointQuery.php b/src/browse/query/ArcanistBrowsePathURIHardpointQuery.php
--- a/src/browse/query/ArcanistBrowsePathURIHardpointQuery.php
+++ b/src/browse/query/ArcanistBrowsePathURIHardpointQuery.php
@@ -55,10 +55,26 @@
$path = Filesystem::readablePath($full_path, $working_root);
}
+ // Define the repository root path, probably useful just for Subversion.
+ $repo_root_path = null;
+ if ($ref->getBranchSupported() === false) {
+ // In software like SVN there is no native branch support. Really.
+ // So, in SVN, we use getBranchName() to get the SVN
+ // "branch", that is, the repository root path, and that's
+ // a root relative path of the repository, like 'trunk' or 'tags/lol'.
+ // Note that the empty string '' is a valid SVN repo root path
+ // that may be returned by "svn info".
+ $repo_root_path = $this
+ ->getRepositoryApi()
+ ->getBranchName();
+ }
+
$params = array(
'path' => $path,
'lines' => $lines,
'branch' => $ref->getBranch(),
+ 'branchSupported' => $ref->getBranchSupported(),
+ 'repoRootPath' => $repo_root_path,
);
$uri = $repository_ref->newBrowseURI($params);
diff --git a/src/browse/ref/ArcanistBrowseRef.php b/src/browse/ref/ArcanistBrowseRef.php
--- a/src/browse/ref/ArcanistBrowseRef.php
+++ b/src/browse/ref/ArcanistBrowseRef.php
@@ -8,8 +8,23 @@
private $token;
private $types = array();
+
+ /**
+ * Branch name (e.g. 'main')
+ * @var string|null
+ */
private $branch;
+ /**
+ * Branch support
+ * This is useful to avoid ambiguity, especially when
+ * the branch attribute is null.
+ * @var bool|null True: supported.
+ * False: unsupported.
+ * Null: unknown support.
+ */
+ private $branchSupported;
+
public function getRefDisplayName() {
return pht('Browse Query "%s"', $this->getToken());
}
@@ -49,13 +64,44 @@
return !$this->types;
}
- public function setBranch($branch) {
- $this->branch = $branch;
+ /**
+ * Set a branch name.
+ * See also setBranchSupported() and use it accordingly.
+ * @param string|null $branch Branch name like 'main' or 'master'
+ * @return self
+ */
+ public function setBranch($branch) {
+ $this->branch = $branch;
+ return $this;
+ }
+
+ /**
+ * Get the branch name.
+ * You may want to also check getBranchSupported() first.
+ * @return string|null Branch name like 'main' or 'master'
+ */
+ public function getBranch() {
+ return $this->branch;
+ }
+
+ /**
+ * Set if the server really supports branches, or not.
+ * Generally we assume yes. In Subversion it's a nope.
+ * @param bool|null $branch_supported
+ * @return self
+ */
+ public function setBranchSupported($branch_supported) {
+ $this->branchSupported = $branch_supported;
return $this;
}
- public function getBranch() {
- return $this->branch;
+ /**
+ * Check if the server really supports branches, or not.
+ * Generally we assume yes. In Subversion it's a nope.
+ * @return bool|null If the info is unknown you may get null.
+ */
+ public function getBranchSupported() {
+ return $this->branchSupported;
}
public function getURIs() {
diff --git a/src/browse/workflow/ArcanistBrowseWorkflow.php b/src/browse/workflow/ArcanistBrowseWorkflow.php
--- a/src/browse/workflow/ArcanistBrowseWorkflow.php
+++ b/src/browse/workflow/ArcanistBrowseWorkflow.php
@@ -100,10 +100,37 @@
$ref->setTypes($types);
}
+ // Some software like Subversion does not really support a branch.
+ // We must inform every "ref" (ArcanistRepositoryRef) about this support,
+ // since they generate the destination URI also thanks to this info.
+ $api_supports_branches = null;
+ if ($this->hasRepositoryAPI()) {
+ $api_supports_branches = $this->getRepositoryAPI()->supportsBranches();
+ foreach ($refs as $ref) {
+ $ref->setBranchSupported($api_supports_branches);
+ }
+ }
+
+ // Allow users to specify a (different) remote branch in which the file is
+ // opened on the web (if branches are supported by the VCS).
$branch = $this->getArgument('branch');
if ($branch) {
- foreach ($refs as $ref) {
- $ref->setBranch($branch);
+ if ($api_supports_branches) {
+ foreach ($refs as $ref) {
+ $ref->setBranch($branch);
+ }
+ } else {
+ // Dear Subversion users,
+ // we have a vague idea about what "--branch" should do in your context,
+ // and that idea makes no sense at the moment. Nothing personal!
+ // https://we.phorge.it/T15541
+ $this->writeWarn(
+ pht('BRANCH OPTION NOT AVAILABLE'),
+ pht(
+ 'Argument "--branch" for "arc browse" is not available '.
+ 'in your software version control. For example, in SVN, '.
+ 'a branch is just a directory with a special meaning for '.
+ 'your team. Please omit the "--branch" argument.'));
}
}
diff --git a/src/ref/ArcanistRepositoryRef.php b/src/ref/ArcanistRepositoryRef.php
--- a/src/ref/ArcanistRepositoryRef.php
+++ b/src/ref/ArcanistRepositoryRef.php
@@ -60,20 +60,47 @@
PhutilTypeSpec::checkMap(
$params,
array(
+ // Path to the file or directory.
'path' => 'optional string|null',
+ // Destination branch.
+ // When the branch is null, we guess the default (e.g. 'master' in git).
'branch' => 'optional string|null',
+ // Branch support capability.
+ // If this is false, you are probably using svn.
+ 'branchSupported' => 'optional bool|null',
+ // Repository root path.
+ // If this is non-null, you are probably using svn. In svn, this path
+ // can be '' for the root, or 'trunk', or 'tags/something' etc.
+ 'repoRootPath' => 'optional string|null',
+ // Lines to be highlighted.
+ // This can be null (no line), line '2' or lines '2-3'.
'lines' => 'optional string|null',
));
+ // Define arguments who do not accept an empty string.
+ // The 'repoRootPath' must not be listed here.
+ $params_with_str_nonempty = array(
+ 'path' => 1,
+ 'branch' => 1,
+ 'lines' => 1,
+ );
+
+ // For all arguments, drop null.
+ // For some arguments, also drop empty strings.
foreach ($params as $key => $value) {
- if ($value === null || !strlen($value)) {
+ if (
+ $value === null
+ || (isset($params_with_str_nonempty[$key]) && !strlen($value))
+ ) {
unset($params[$key]);
}
}
$defaults = array(
'path' => '/',
- 'branch' => $this->getDefaultBranch(),
+ 'branch' => null,
+ 'branchSupported' => null,
+ 'repoRootPath' => null,
'lines' => null,
);
@@ -82,20 +109,40 @@
$uri_base = coalesce($this->browseURI, '');
$uri_base = rtrim($uri_base, '/');
- $uri_branch = phutil_escape_uri_path_component($params['branch']);
+ // Build the Subversion repo root path.
+ // In git, this is an empty string.
+ // In svn, this is 'trunk/' or '/' etc.
+ $uri_root = '';
+ if ($params['repoRootPath'] !== null) {
+ $uri_root = trim($params['repoRootPath'], '/');
+ $uri_root = phutil_escape_uri($uri_root); // Don't escape slashes.
+ if ($uri_root !== '') {
+ $uri_root .= '/';
+ }
+ }
+
+ // Build the branch part of the URI.
+ // In git, this becomes 'master/', 'main/', etc.
+ // In svn, this is an empty string.
+ $uri_branch = '';
+ if ($params['branchSupported']) {
+ $uri_branch = coalesce($params['branch'], $this->getDefaultBranch());
+ $uri_branch = phutil_escape_uri_path_component($uri_branch); // Escape '/'
+ $uri_branch .= '/';
+ }
$uri_path = ltrim($params['path'], '/');
$uri_path = phutil_escape_uri($uri_path);
$uri_lines = null;
if ($params['lines']) {
+ // We should encourage an #anchor, not a dollar.
+ // https://we.phorge.it/T15670
$uri_lines = '$'.phutil_escape_uri($params['lines']);
}
- // TODO: This construction, which includes a branch, is probably wrong for
- // Subversion.
-
- return "{$uri_base}/browse/{$uri_branch}/{$uri_path}{$uri_lines}";
+ // This construction supports both git and Subversion.
+ return "{$uri_base}/browse/{$uri_root}{$uri_branch}{$uri_path}{$uri_lines}";
}
public function getDefaultBranch() {
diff --git a/src/repository/api/ArcanistRepositoryAPI.php b/src/repository/api/ArcanistRepositoryAPI.php
--- a/src/repository/api/ArcanistRepositoryAPI.php
+++ b/src/repository/api/ArcanistRepositoryAPI.php
@@ -567,6 +567,20 @@
/* -( Base Commits )------------------------------------------------------- */
+ /**
+ * In some environments (like Subversion) the concept of "branch"
+ * is not really integrated in any standard server and it's just
+ * a path on your local filesystem. Like "trunk/" etc.
+ *
+ * @return bool True if the branch is supported by this API.
+ */
+ public function supportsBranches() {
+ // Assume a decent default.
+ // In the future we may want to make this method abstract. But not now,
+ // to avoid breaking changes in alien class extensions.
+ return true;
+ }
+
abstract public function supportsCommitRanges();
final public function setBaseCommit($symbolic_commit) {
diff --git a/src/repository/api/ArcanistSubversionAPI.php b/src/repository/api/ArcanistSubversionAPI.php
--- a/src/repository/api/ArcanistSubversionAPI.php
+++ b/src/repository/api/ArcanistSubversionAPI.php
@@ -624,6 +624,15 @@
return false;
}
+ /**
+ * @return bool
+ */
+ public function supportsBranches() {
+ // Even if you are the best SVN salesman, a little cute
+ // directory cannot really be considered a "branch" feature.
+ return false;
+ }
+
public function supportsCommitRanges() {
return false;
}
diff --git a/src/utils/utils.php b/src/utils/utils.php
--- a/src/utils/utils.php
+++ b/src/utils/utils.php
@@ -55,7 +55,7 @@
*
* For example, `idxv($dict, array('a', 'b', 'c'))` accesses the key at
* `$dict['a']['b']['c']`, if it exists. If it does not, or any intermediate
- * value is not itself an array, it returns the defualt value.
+ * value is not itself an array, it returns the default value.
*
* @param array $map Array to access.
* @param list<string> $path List of keys to access, in sequence.
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Wed, May 7, 02:43 (19 h, 8 m ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1503720
Default Alt Text
D25823.1746585813.diff (10 KB)
Attached To
Mode
D25823: arc browse: add support for Subversion repos
Attached
Detach File
Event Timeline
Log In to Comment