From 8b4b614d15f8349bfc445375c8f0b609ba4a3f0a Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 19 Jan 2012 11:49:51 -0800 Subject: [PATCH] Show all branches a commit appears on Summary: We show the contextual branch (always the repository default branch) when viewing a commit. Instead, show all branches the commit appears on. Also pull some of the duplicated DiffusionXQuery stuff into a DiffusionQuery base class, I'll do a followup to reduce more duplication. Test Plan: Looked at a commit in Git. My HG and SVN setups are a little borked so I kind of faked tests in them -- I'm fixing them now. Reviewers: btrahan, jungejason, fratrik Reviewed By: btrahan CC: aran Maniphest Tasks: T768 Differential Revision: https://secure.phabricator.com/D1458 --- src/__phutil_library_map__.php | 9 +++ .../controller/base/DiffusionController.php | 13 +++-- .../commit/DiffusionCommitController.php | 14 ++++- .../diffusion/controller/commit/__init__.php | 1 + .../diffusion/query/base/DiffusionQuery.php | 56 +++++++++++++++++++ .../diffusion/query/base/__init__.php | 14 +++++ .../contains/base/DiffusionContainsQuery.php | 30 ++++++++++ .../query/contains/base/__init__.php | 12 ++++ .../git/DiffusionGitContainsQuery.php | 34 +++++++++++ .../diffusion/query/contains/git/__init__.php | 14 +++++ .../DiffusionMercurialContainsQuery.php | 28 ++++++++++ .../query/contains/mercurial/__init__.php | 12 ++++ .../svn/DiffusionSVNContainsQuery.php | 26 +++++++++ .../diffusion/query/contains/svn/__init__.php | 12 ++++ 14 files changed, 268 insertions(+), 7 deletions(-) create mode 100644 src/applications/diffusion/query/base/DiffusionQuery.php create mode 100644 src/applications/diffusion/query/base/__init__.php create mode 100644 src/applications/diffusion/query/contains/base/DiffusionContainsQuery.php create mode 100644 src/applications/diffusion/query/contains/base/__init__.php create mode 100644 src/applications/diffusion/query/contains/git/DiffusionGitContainsQuery.php create mode 100644 src/applications/diffusion/query/contains/git/__init__.php create mode 100644 src/applications/diffusion/query/contains/mercurial/DiffusionMercurialContainsQuery.php create mode 100644 src/applications/diffusion/query/contains/mercurial/__init__.php create mode 100644 src/applications/diffusion/query/contains/svn/DiffusionSVNContainsQuery.php create mode 100644 src/applications/diffusion/query/contains/svn/__init__.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index fbbd7e1a01..1812e8915a 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -262,6 +262,7 @@ phutil_register_library_map(array( 'DiffusionCommitChangeTableView' => 'applications/diffusion/view/commitchangetable', 'DiffusionCommitController' => 'applications/diffusion/controller/commit', 'DiffusionCommitListController' => 'applications/diffusion/controller/commitlist', + 'DiffusionContainsQuery' => 'applications/diffusion/query/contains/base', 'DiffusionController' => 'applications/diffusion/controller/base', 'DiffusionDiffController' => 'applications/diffusion/controller/diff', 'DiffusionDiffQuery' => 'applications/diffusion/query/diff/base', @@ -270,6 +271,7 @@ phutil_register_library_map(array( 'DiffusionGitBranchQuery' => 'applications/diffusion/query/branch/git', 'DiffusionGitBranchQueryTestCase' => 'applications/diffusion/query/branch/git/__tests__', 'DiffusionGitBrowseQuery' => 'applications/diffusion/query/browse/git', + 'DiffusionGitContainsQuery' => 'applications/diffusion/query/contains/git', 'DiffusionGitDiffQuery' => 'applications/diffusion/query/diff/git', 'DiffusionGitFileContentQuery' => 'applications/diffusion/query/filecontent/git', 'DiffusionGitHistoryQuery' => 'applications/diffusion/query/history/git', @@ -283,6 +285,7 @@ phutil_register_library_map(array( 'DiffusionLastModifiedQuery' => 'applications/diffusion/query/lastmodified/base', 'DiffusionMercurialBranchQuery' => 'applications/diffusion/query/branch/mercurial', 'DiffusionMercurialBrowseQuery' => 'applications/diffusion/query/browse/mercurial', + 'DiffusionMercurialContainsQuery' => 'applications/diffusion/query/contains/mercurial', 'DiffusionMercurialDiffQuery' => 'applications/diffusion/query/diff/mercurial', 'DiffusionMercurialFileContentQuery' => 'applications/diffusion/query/filecontent/mercurial', 'DiffusionMercurialHistoryQuery' => 'applications/diffusion/query/history/mercurial', @@ -294,9 +297,11 @@ phutil_register_library_map(array( 'DiffusionPathIDQuery' => 'applications/diffusion/query/pathid/base', 'DiffusionPathQueryTestCase' => 'applications/diffusion/query/pathid/base/__tests__', 'DiffusionPathValidateController' => 'applications/diffusion/controller/pathvalidate', + 'DiffusionQuery' => 'applications/diffusion/query/base', 'DiffusionRepositoryController' => 'applications/diffusion/controller/repository', 'DiffusionRepositoryPath' => 'applications/diffusion/data/repositorypath', 'DiffusionRequest' => 'applications/diffusion/request/base', + 'DiffusionSVNContainsQuery' => 'applications/diffusion/query/contains/svn', 'DiffusionSvnBrowseQuery' => 'applications/diffusion/query/browse/svn', 'DiffusionSvnDiffQuery' => 'applications/diffusion/query/diff/svn', 'DiffusionSvnFileContentQuery' => 'applications/diffusion/query/filecontent/svn', @@ -997,11 +1002,13 @@ phutil_register_library_map(array( 'DiffusionCommitChangeTableView' => 'DiffusionView', 'DiffusionCommitController' => 'DiffusionController', 'DiffusionCommitListController' => 'DiffusionController', + 'DiffusionContainsQuery' => 'DiffusionQuery', 'DiffusionController' => 'PhabricatorController', 'DiffusionDiffController' => 'DiffusionController', 'DiffusionGitBranchQuery' => 'DiffusionBranchQuery', 'DiffusionGitBranchQueryTestCase' => 'PhabricatorTestCase', 'DiffusionGitBrowseQuery' => 'DiffusionBrowseQuery', + 'DiffusionGitContainsQuery' => 'DiffusionContainsQuery', 'DiffusionGitDiffQuery' => 'DiffusionDiffQuery', 'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery', 'DiffusionGitHistoryQuery' => 'DiffusionHistoryQuery', @@ -1013,6 +1020,7 @@ phutil_register_library_map(array( 'DiffusionLastModifiedController' => 'DiffusionController', 'DiffusionMercurialBranchQuery' => 'DiffusionBranchQuery', 'DiffusionMercurialBrowseQuery' => 'DiffusionBrowseQuery', + 'DiffusionMercurialContainsQuery' => 'DiffusionContainsQuery', 'DiffusionMercurialDiffQuery' => 'DiffusionDiffQuery', 'DiffusionMercurialFileContentQuery' => 'DiffusionFileContentQuery', 'DiffusionMercurialHistoryQuery' => 'DiffusionHistoryQuery', @@ -1022,6 +1030,7 @@ phutil_register_library_map(array( 'DiffusionPathQueryTestCase' => 'PhabricatorTestCase', 'DiffusionPathValidateController' => 'DiffusionController', 'DiffusionRepositoryController' => 'DiffusionController', + 'DiffusionSVNContainsQuery' => 'DiffusionContainsQuery', 'DiffusionSvnBrowseQuery' => 'DiffusionBrowseQuery', 'DiffusionSvnDiffQuery' => 'DiffusionDiffQuery', 'DiffusionSvnFileContentQuery' => 'DiffusionFileContentQuery', diff --git a/src/applications/diffusion/controller/base/DiffusionController.php b/src/applications/diffusion/controller/base/DiffusionController.php index 0d5fa73c0c..d8c55106b7 100644 --- a/src/applications/diffusion/controller/base/DiffusionController.php +++ b/src/applications/diffusion/controller/base/DiffusionController.php @@ -1,7 +1,7 @@ getCallsign(); $repository_name = phutil_escape_html($repository->getName()).' Repository'; - $branch_name = $drequest->getBranch(); - if ($branch_name) { - $repository_name .= ' ('.phutil_escape_html($branch_name).')'; + if (empty($spec['commit'])) { + $branch_name = $drequest->getBranch(); + if ($branch_name) { + $repository_name .= ' ('.phutil_escape_html($branch_name).')'; + } } - $branch_uri = $drequest->getBranchURIComponent($drequest->getBranch()); - if (empty($spec['view']) && empty($spec['commit'])) { $crumb_list[] = $repository_name; return $crumb_list; @@ -220,6 +220,7 @@ abstract class DiffusionController extends PhabricatorController { return $crumb_list; } + $branch_uri = $drequest->getBranchURIComponent($drequest->getBranch()); $view_root_uri = "/diffusion/{$callsign}/{$view}/{$branch_uri}"; $jump_href = $view_root_uri; diff --git a/src/applications/diffusion/controller/commit/DiffusionCommitController.php b/src/applications/diffusion/controller/commit/DiffusionCommitController.php index 3a0125d74e..5a4829612e 100644 --- a/src/applications/diffusion/controller/commit/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/commit/DiffusionCommitController.php @@ -1,7 +1,7 @@ renderLink(); } + $request = $this->getDiffusionRequest(); + + $contains = DiffusionContainsQuery::newFromDiffusionRequest($request); + $branches = $contains->loadContainingBranches(); + + if ($branches) { + // TODO: Separate these into 'tracked' and other; link tracked branches. + $branches = implode(', ', array_keys($branches)); + $branches = phutil_escape_html($branches); + $props['Branches'] = $branches; + } + $rows = array(); foreach ($props as $key => $value) { $rows[] = diff --git a/src/applications/diffusion/controller/commit/__init__.php b/src/applications/diffusion/controller/commit/__init__.php index 1462f1b18e..e74c53d5e0 100644 --- a/src/applications/diffusion/controller/commit/__init__.php +++ b/src/applications/diffusion/controller/commit/__init__.php @@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'applications/differential/constants/change phutil_require_module('phabricator', 'applications/differential/view/changesetlistview'); phutil_require_module('phabricator', 'applications/diffusion/controller/base'); phutil_require_module('phabricator', 'applications/diffusion/data/pathchange'); +phutil_require_module('phabricator', 'applications/diffusion/query/contains/base'); phutil_require_module('phabricator', 'applications/diffusion/query/pathchange/base'); phutil_require_module('phabricator', 'applications/diffusion/view/commitchangetable'); phutil_require_module('phabricator', 'applications/markup/engine'); diff --git a/src/applications/diffusion/query/base/DiffusionQuery.php b/src/applications/diffusion/query/base/DiffusionQuery.php new file mode 100644 index 0000000000..fe3116650b --- /dev/null +++ b/src/applications/diffusion/query/base/DiffusionQuery.php @@ -0,0 +1,56 @@ + + } + + protected static function newQueryObject( + $base_class, + DiffusionRequest $request) { + + $repository = $request->getRepository(); + + $map = array( + PhabricatorRepositoryType::REPOSITORY_TYPE_GIT => 'Git', + PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL => 'Mercurial', + PhabricatorRepositoryType::REPOSITORY_TYPE_SVN => 'SVN', + ); + + $name = idx($map, $repository->getVersionControlSystem()); + if (!$name) { + throw new Exception("Unsupported VCS!"); + } + + $class = str_replace('Diffusion', 'Diffusion'.$name, $base_class); + $obj = new $class(); + $obj->request = $request; + + return $obj; + } + + final protected function getRequest() { + return $this->request; + } + + abstract protected function executeQuery(); +} diff --git a/src/applications/diffusion/query/base/__init__.php b/src/applications/diffusion/query/base/__init__.php new file mode 100644 index 0000000000..96c180b523 --- /dev/null +++ b/src/applications/diffusion/query/base/__init__.php @@ -0,0 +1,14 @@ +executeQuery(); + } + +} diff --git a/src/applications/diffusion/query/contains/base/__init__.php b/src/applications/diffusion/query/contains/base/__init__.php new file mode 100644 index 0000000000..879029fe29 --- /dev/null +++ b/src/applications/diffusion/query/contains/base/__init__.php @@ -0,0 +1,12 @@ +getRequest(); + $repository = $request->getRepository(); + + list($contains) = $repository->execxLocalCommand( + 'branch -r --verbose --no-abbrev --contains %s', + $request->getCommit()); + + return DiffusionGitBranchQuery::parseGitRemoteBranchOutput( + $contains, + DiffusionBranchInformation::DEFAULT_GIT_REMOTE); + } + +} diff --git a/src/applications/diffusion/query/contains/git/__init__.php b/src/applications/diffusion/query/contains/git/__init__.php new file mode 100644 index 0000000000..9c670e2074 --- /dev/null +++ b/src/applications/diffusion/query/contains/git/__init__.php @@ -0,0 +1,14 @@ +