Page Menu
Home
Phorge
Search
Configure Global Search
Log In
Files
F2608694
D25823.1731191155.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
9 KB
Referenced Files
None
Subscribers
None
D25823.1731191155.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 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,25 @@
$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.
+ $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 +123,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.
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sat, Nov 9, 22:25 (19 h, 40 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
963729
Default Alt Text
D25823.1731191155.diff (9 KB)
Attached To
Mode
D25823: arc browse: add support to Subversion repos
Attached
Detach File
Event Timeline
Log In to Comment