Page MenuHomePhorge
Files F2680761

D25823.1734621775.diff
No OneTemporary
Actions

D25823.1734621775.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
@@ -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,33 @@
$path = Filesystem::readablePath($full_path, $working_root);
}
+ // Guess the branch. Easy for git. Not so easy for SVN.
+ $branch = null;
+ if ($api && $api->isBranchJustFilesystemPath()) {
+ // In SVN, the method getBranchName() returns the SVN
+ // repository root path. Here some valid expected values:
+ // '/trunk'
+ // '/really/whatever/path/123'
+ // ''
+ // 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 = $this->getRepositoryApi()->getBranchName();
+ if ($branch === '') {
+ $branch = '/';
+ }
+ } else {
+ // The user may specify the preferred remote branch like "master".
+ $branch = $ref->getBranch();
+ }
+
$params = array(
'path' => $path,
'lines' => $lines,
- 'branch' => $ref->getBranch(),
+ 'branch' => $branch,
);
$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
@@ -82,7 +82,20 @@
$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.
+ // It's important to end with a slash.
+ // git: we get 'master' and we want 'master/'
+ // svn: we get '/trunk' and we want 'trunk/'
+ // svn: we get '/' and we want '/'
+ // svn: we get '' and we want '/'
+ $uri_branch = '';
+ if ($params['branch'] !== '') {
+ $uri_branch = trim($params['branch'], '/');
+ $uri_branch = phutil_escape_uri_path_component($uri_branch);
+ if ($uri_branch !== '') {
+ $uri_branch .= '/';
+ }
+ }
$uri_path = ltrim($params['path'], '/');
$uri_path = phutil_escape_uri($uri_path);
@@ -92,10 +105,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
Thu, Dec 19, 15:22 (9 h, 34 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1015132
Default Alt Text
D25823.1734621775.diff (4 KB)

Event Timeline

Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0