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 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(); // lol - not really a branch
+ }
+
$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,35 @@
$ref->setTypes($types);
}
+ // Some software like Subversion does not really support a branch.
+ $api_supports_branches = null;
+ if ($this->hasRepositoryAPI()) {
+ $api_supports_branches = $this->getRepositoryAPI()->supportsBranches();
+ foreach ($refs as $ref) {
+ $ref->setBranchSupported($api_supports_branches);
+ }
+ }
+
+ // Allow the user to choose a remote branch, so the specified file
+ // is opened there on the web, if it has sense to do so.
$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
@@ -62,18 +62,31 @@
array(
'path' => 'optional string|null',
'branch' => 'optional string|null',
+ 'branchSupported' => 'optional bool|null',
+ 'repoRootPath' => 'optional string|null', // This loves an empty string.
'lines' => 'optional string|null',
));
+ // For all arguments, drop NULL.
+ // For some arguments (indexed here), drop empty strings.
+ $params_with_str_nonempty = array(
+ 'path' => 1,
+ 'branch' => 1,
+ 'lines' => 1,
+ );
foreach ($params as $key => $value) {
- if ($value === null || !strlen($value)) {
+ if ($value === null) {
+ unset($params[$key]);
+ } else if (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,7 +95,27 @@
$uri_base = coalesce($this->browseURI, '');
$uri_base = rtrim($uri_base, '/');
- $uri_branch = phutil_escape_uri_path_component($params['branch']);
+ // Build the repo root path of the URI.
+ // This is a feature used by Subversion to only clone a single
+ // sub-path like "trunk" or "tags/foo/whatever".
+ // It's unclear whenever the original value may end with a slash.
+ // Anyway, the final URI should end with just one slash (or be empty).
+ $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. This usually becomes 'master/'.
+ $uri_branch = '';
+ if ($params['branchSupported']) {
+ $uri_branch = coalesce($params['branch'], $this->getDefaultBranch());
+ $uri_branch = phutil_escape_uri_path_component($uri_branch);
+ $uri_branch .= '/';
+ }
$uri_path = ltrim($params['path'], '/');
$uri_path = phutil_escape_uri($uri_path);
@@ -92,10 +125,8 @@
$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.