Page MenuHomePhorge

D25823.1731147022.diff
No OneTemporary

D25823.1731147022.diff

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
@@ -6,6 +6,8 @@
const BROWSETYPE = 'path';
public function loadHardpoint(array $refs, $hardpoint) {
+ $api = $this->getRepositoryApi();
+
$refs = $this->getRefsWithSupportedTypes($refs);
if (!$refs) {
yield $this->yieldMap(array());
@@ -55,10 +57,34 @@
$path = Filesystem::readablePath($full_path, $working_root);
}
+ // The user may specify the preferred remote branch like "master".
+ $branch = $ref->getBranch();
+ $branch_is_path = $api && $api->isBranchJustFilesystemPath();
+ if ($branch === null && $branch_is_path) {
+ // Subversion does not really support branches.
+ // So, in SVN, your branch is just your directory.
+ // So, in SVN, we use getBranchName() to get the SVN
+ // repository root path. Here some valid expected values:
+ // '/really/whatever/path/123' -> '/really/whatever/path/123'
+ // '/trunk' -> '/trunk'
+ // '/' -> ''
+ // Yes. The empty string is a valid SVN repository root.
+ // Almost surely, an empty string may cause extra confusion
+ // to downstream logics, as they tend to propose nice defaults,
+ // including the default branch name "master".
+ // So, to assure this empty value is respected, we promote the
+ // empty string to a single slash - that makes sense.
+ $branch = $api->getBranchName();
+ if ($branch === '') {
+ $branch = '/';
+ }
+ }
+
$params = array(
'path' => $path,
'lines' => $lines,
- 'branch' => $ref->getBranch(),
+ 'branch' => $branch,
+ 'branchIsPath' => $branch_is_path,
);
$uri = $repository_ref->newBrowseURI($params);
diff --git a/src/ref/ArcanistRepositoryRef.php b/src/ref/ArcanistRepositoryRef.php
--- a/src/ref/ArcanistRepositoryRef.php
+++ b/src/ref/ArcanistRepositoryRef.php
@@ -62,6 +62,7 @@
array(
'path' => 'optional string|null',
'branch' => 'optional string|null',
+ 'branchIsPath' => 'optional bool|false',
'lines' => 'optional string|null',
));
@@ -74,6 +75,7 @@
$defaults = array(
'path' => '/',
'branch' => $this->getDefaultBranch(),
+ 'branchIsPath' => false,
'lines' => null,
);
@@ -82,7 +84,23 @@
$uri_base = coalesce($this->browseURI, '');
$uri_base = rtrim($uri_base, '/');
- $uri_branch = phutil_escape_uri_path_component($params['branch']);
+ // Build the branch part of the URI.
+ // git: we get 'master' and we want 'master/'
+ // svn: we get '/trunk' and we want 'trunk/'
+ // svn: we get '/' and we want '/'
+ // all: we get '' and we want ''
+ $uri_branch = trim($params['branch'], '/');
+
+ // In git the branch cannot be a path. So, escape slashes.
+ // In svn the branch can be "/tags/something". So, do not escape.
+ if (!$params['branchIsPath']) {
+ $uri_branch = phutil_escape_uri_path_component($uri_branch);
+ }
+
+ // If the branch is available, make sure to end with slash.
+ if ($uri_branch !== '') {
+ $uri_branch .= '/';
+ }
$uri_path = ltrim($params['path'], '/');
$uri_path = phutil_escape_uri($uri_path);
@@ -92,10 +110,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_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
@@ -348,6 +348,22 @@
array $query);
abstract public function getRemoteURI();
+ /**
+ * In some environments like Subversion the branch
+ * does not really exist but it's just a path
+ * on your local filesystem. Like "trunk/" etc.
+ *
+ * @return bool
+ */
+ public function isBranchJustFilesystemPath() {
+ // In any decent and modern software version control
+ // system, including git (and Mercurial?) the branch
+ // is NOT just a path on the filesystem, but something
+ // more structured and partially unrelated with your
+ // working directory.
+ return false;
+ }
+
public function getChangedFiles($since_commit) {
throw new ArcanistCapabilityNotSupportedException($this);
}
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
@@ -243,6 +243,17 @@
return $this;
}
+ /**
+ * In Subversion the branch does not really exist.
+ * Instead there is a particular path on your local
+ * filesystem, like "trunk/".
+ *
+ * @return bool
+ */
+ public function isBranchJustFilesystemPath() {
+ return true;
+ }
+
public function getBranchName() {
$info = $this->getSVNInfo('/');
$repo_root = idx($info, 'Repository Root');

File Metadata

Mime Type
text/plain
Expires
Sat, Nov 9, 10:10 (12 h, 17 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
962861
Default Alt Text
D25823.1731147022.diff (5 KB)

Event Timeline