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');