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