Improve tag support in Diffusion

Summary:
  - When viewing a commit, show its tags.
  - For commits with many tags, show a list of all tags on the tag list interface.
  - Improve some handling of symbolic references.
  - When tags contain content, show it on the browse view reached by clicking the tag name.

Test Plan: Looked at commits with and without tags, clicked "More tags...", clicked tag names.

Reviewers: btrahan, vrana, davidreuss, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1130

Differential Revision: https://secure.phabricator.com/D2290

src/__phutil_library_map__.php

@@ -99,6 +99,7 @@ phutil_register_library_map(array(
     'AphrontTokenizerTemplateView' => 'view/control/tokenizer',
     'AphrontTypeaheadTemplateView' => 'view/control/typeahead',
     'AphrontURIMapper' => 'aphront/mapper',
+    'AphrontUsageException' => 'aphront/exception/usage',
     'AphrontView' => 'view/base',
     'AphrontWebpageResponse' => 'aphront/response/webpage',
     'AphrontWriteGuard' => 'aphront/writeguard',
@@ -303,6 +304,7 @@ phutil_register_library_map(array(
     'DiffusionCommitChangeTableView' => 'applications/diffusion/view/commitchangetable',
     'DiffusionCommitController' => 'applications/diffusion/controller/commit',
     'DiffusionCommitParentsQuery' => 'applications/diffusion/query/parents/base',
+    'DiffusionCommitTagsQuery' => 'applications/diffusion/query/committags/base',
     'DiffusionContainsQuery' => 'applications/diffusion/query/contains/base',
     'DiffusionController' => 'applications/diffusion/controller/base',
     'DiffusionDiffController' => 'applications/diffusion/controller/diff',
@@ -315,6 +317,7 @@ phutil_register_library_map(array(
     'DiffusionGitBranchQueryTestCase' => 'applications/diffusion/query/branch/git/__tests__',
     'DiffusionGitBrowseQuery' => 'applications/diffusion/query/browse/git',
     'DiffusionGitCommitParentsQuery' => 'applications/diffusion/query/parents/git',
+    'DiffusionGitCommitTagsQuery' => 'applications/diffusion/query/committags/git',
     'DiffusionGitContainsQuery' => 'applications/diffusion/query/contains/git',
     'DiffusionGitDiffQuery' => 'applications/diffusion/query/diff/git',
     'DiffusionGitFileContentQuery' => 'applications/diffusion/query/filecontent/git',
@@ -334,6 +337,7 @@ phutil_register_library_map(array(
     'DiffusionMercurialBranchQuery' => 'applications/diffusion/query/branch/mercurial',
     'DiffusionMercurialBrowseQuery' => 'applications/diffusion/query/browse/mercurial',
     'DiffusionMercurialCommitParentsQuery' => 'applications/diffusion/query/parents/mercurial',
+    'DiffusionMercurialCommitTagsQuery' => 'applications/diffusion/query/committags/mercurial',
     'DiffusionMercurialContainsQuery' => 'applications/diffusion/query/contains/mercurial',
     'DiffusionMercurialDiffQuery' => 'applications/diffusion/query/diff/mercurial',
     'DiffusionMercurialFileContentQuery' => 'applications/diffusion/query/filecontent/mercurial',
@@ -359,6 +363,7 @@ phutil_register_library_map(array(
     'DiffusionRequest' => 'applications/diffusion/request/base',
     'DiffusionSvnBrowseQuery' => 'applications/diffusion/query/browse/svn',
     'DiffusionSvnCommitParentsQuery' => 'applications/diffusion/query/parents/svn',
+    'DiffusionSvnCommitTagsQuery' => 'applications/diffusion/query/committags/svn',
     'DiffusionSvnContainsQuery' => 'applications/diffusion/query/contains/svn',
     'DiffusionSvnDiffQuery' => 'applications/diffusion/query/diff/svn',
     'DiffusionSvnFileContentQuery' => 'applications/diffusion/query/filecontent/svn',
@@ -1105,6 +1110,7 @@ phutil_register_library_map(array(
     'AphrontTableView' => 'AphrontView',
     'AphrontTokenizerTemplateView' => 'AphrontView',
     'AphrontTypeaheadTemplateView' => 'AphrontView',
+    'AphrontUsageException' => 'AphrontException',
     'AphrontWebpageResponse' => 'AphrontResponse',
     'CelerityResourceController' => 'AphrontController',
     'CelerityResourceGraph' => 'AbstractDirectedGraph',
@@ -1269,6 +1275,7 @@ phutil_register_library_map(array(
     'DiffusionCommitChangeTableView' => 'DiffusionView',
     'DiffusionCommitController' => 'DiffusionController',
     'DiffusionCommitParentsQuery' => 'DiffusionQuery',
+    'DiffusionCommitTagsQuery' => 'DiffusionQuery',
     'DiffusionContainsQuery' => 'DiffusionQuery',
     'DiffusionController' => 'PhabricatorController',
     'DiffusionDiffController' => 'DiffusionController',
@@ -1280,6 +1287,7 @@ phutil_register_library_map(array(
     'DiffusionGitBranchQueryTestCase' => 'PhabricatorTestCase',
     'DiffusionGitBrowseQuery' => 'DiffusionBrowseQuery',
     'DiffusionGitCommitParentsQuery' => 'DiffusionCommitParentsQuery',
+    'DiffusionGitCommitTagsQuery' => 'DiffusionCommitTagsQuery',
     'DiffusionGitContainsQuery' => 'DiffusionContainsQuery',
     'DiffusionGitDiffQuery' => 'DiffusionDiffQuery',
     'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery',
@@ -1299,6 +1307,7 @@ phutil_register_library_map(array(
     'DiffusionMercurialBranchQuery' => 'DiffusionBranchQuery',
     'DiffusionMercurialBrowseQuery' => 'DiffusionBrowseQuery',
     'DiffusionMercurialCommitParentsQuery' => 'DiffusionCommitParentsQuery',
+    'DiffusionMercurialCommitTagsQuery' => 'DiffusionCommitTagsQuery',
     'DiffusionMercurialContainsQuery' => 'DiffusionContainsQuery',
     'DiffusionMercurialDiffQuery' => 'DiffusionDiffQuery',
     'DiffusionMercurialFileContentQuery' => 'DiffusionFileContentQuery',
@@ -1316,6 +1325,7 @@ phutil_register_library_map(array(
     'DiffusionRepositoryController' => 'DiffusionController',
     'DiffusionSvnBrowseQuery' => 'DiffusionBrowseQuery',
     'DiffusionSvnCommitParentsQuery' => 'DiffusionCommitParentsQuery',
+    'DiffusionSvnCommitTagsQuery' => 'DiffusionCommitTagsQuery',
     'DiffusionSvnContainsQuery' => 'DiffusionContainsQuery',
     'DiffusionSvnDiffQuery' => 'DiffusionDiffQuery',
     'DiffusionSvnFileContentQuery' => 'DiffusionFileContentQuery',

src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php

@@ -504,6 +504,22 @@ class AphrontDefaultApplicationConfiguration
       return $response;
     }
 
+    if ($ex instanceof AphrontUsageException) {
+      $error = new AphrontErrorView();
+      $error->setTitle(phutil_escape_html($ex->getTitle()));
+      $error->appendChild(phutil_escape_html($ex->getMessage()));
+
+      $view = new PhabricatorStandardPageView();
+      $view->setRequest($this->getRequest());
+      $view->appendChild($error);
+
+      $response = new AphrontWebpageResponse();
+      $response->setContent($view->render());
+
+      return $response;
+    }
+
+
     // Always log the unhandled exception.
     phlog($ex);
 

src/aphront/default/configuration/__init__.php

@@ -17,6 +17,7 @@ phutil_require_module('phabricator', 'applications/people/storage/user');
 phutil_require_module('phabricator', 'infrastructure/env');
 phutil_require_module('phabricator', 'view/control/table');
 phutil_require_module('phabricator', 'view/dialog');
+phutil_require_module('phabricator', 'view/form/error');
 phutil_require_module('phabricator', 'view/page/standard');
 
 phutil_require_module('phutil', 'error');

src/aphront/exception/usage/AphrontUsageException.php

@@ -0,0 +1,37 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * These exceptions represent user error, and are not logged.
+ *
+ * @concrete-extensible
+ */
+class AphrontUsageException extends AphrontException {
+
+  private $title;
+
+  public function __construct($title, $message) {
+    $this->title = $title;
+    parent::__construct($message);
+  }
+
+  public function getTitle() {
+    return $this->title;
+  }
+
+}

src/aphront/exception/usage/__init__.php

@@ -0,0 +1,12 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'aphront/exception/base');
+
+
+phutil_require_source('AphrontUsageException.php');

src/applications/diffusion/controller/base/DiffusionController.php

@@ -213,13 +213,27 @@ abstract class DiffusionController extends PhabricatorController {
       $repository_name);
 
     $raw_commit = $drequest->getRawCommit();
-    if ($spec['commit']) {
-      $crumb_list[] = "r{$callsign}{$raw_commit}";
+
+    if ($spec['tags']) {
+      if ($spec['commit']) {
+        $crumb_list[] = "Tags for ".phutil_render_tag(
+          'a',
+          array(
+            'href' => $drequest->generateURI(
+              array(
+                'action' => 'commit',
+                'commit' => $raw_commit,
+              )),
+          ),
+          phutil_escape_html("r{$callsign}{$raw_commit}"));
+      } else {
+        $crumb_list[] = 'Tags';
+      }
       return $crumb_list;
     }
 
-    if ($spec['tags']) {
-      $crumb_list[] = 'Tags';
+    if ($spec['commit']) {
+      $crumb_list[] = "r{$callsign}{$raw_commit}";
       return $crumb_list;
     }
 

src/applications/diffusion/controller/browse/DiffusionBrowseController.php

@@ -33,6 +33,17 @@ final class DiffusionBrowseController extends DiffusionController {
         'view'   => 'browse',
       ));
 
+    if ($drequest->getTagContent()) {
+      $title = 'Tag: '.$drequest->getSymbolicCommit();
+
+      $tag_view = new AphrontPanelView();
+      $tag_view->setHeader(phutil_escape_html($title));
+      $tag_view->appendChild(
+        $this->markupText($drequest->getTagContent()));
+
+      $content[] = $tag_view;
+    }
+
     if (!$results) {
 
       if ($browse_query->getReasonForEmptyResultSet() ==
@@ -88,16 +99,7 @@ final class DiffusionBrowseController extends DiffusionController {
           $readme_content = nl2br($readme_content);
         } else {
           // Markup extensionless files as remarkup so we get links and such.
-
-          $engine = PhabricatorMarkupEngine::newDiffusionMarkupEngine();
-          $readme_content = $engine->markupText($readme_content);
-
-          $readme_content = phutil_render_tag(
-            'div',
-            array(
-              'class' => 'phabricator-remarkup',
-            ),
-            $readme_content);
+          $readme_content = $this->markupText($readme_content);
         }
 
         $readme_panel = new AphrontPanelView();
@@ -126,8 +128,25 @@ final class DiffusionBrowseController extends DiffusionController {
     return $this->buildStandardPageResponse(
       $nav,
       array(
-        'title' => basename($drequest->getPath()),
+        'title' => array(
+          nonempty(basename($drequest->getPath()), '/'),
+          $drequest->getRepository()->getCallsign().' Repository',
+        ),
       ));
   }
 
+  private function markupText($text) {
+    $engine = PhabricatorMarkupEngine::newDiffusionMarkupEngine();
+    $text = $engine->markupText($text);
+
+    $text = phutil_render_tag(
+      'div',
+      array(
+        'class' => 'phabricator-remarkup',
+      ),
+      $text);
+
+    return $text;
+  }
+
 }

src/applications/diffusion/controller/commit/DiffusionCommitController.php

@@ -351,6 +351,12 @@ final class DiffusionCommitController extends DiffusionController {
       $props['Branches'] = $branches;
     }
 
+
+    $tags = $this->renderTags($request);
+    if ($tags) {
+      $props['Tags'] = $tags;
+    }
+
     if ($task_phids) {
       $task_list = array();
       foreach ($task_phids as $phid) {
@@ -720,5 +726,49 @@ final class DiffusionCommitController extends DiffusionController {
     return $action_list;
   }
 
+  private function renderTags(DiffusionRequest $request) {
+    $tag_limit = 10;
+
+    $tag_query = DiffusionCommitTagsQuery::newFromDiffusionRequest($request);
+    $tag_query->setLimit($tag_limit + 1);
+    $tags = $tag_query->loadTags();
+
+    if (!$tags) {
+      return null;
+    }
+
+    $has_more_tags = (count($tags) > $tag_limit);
+    $tags = array_slice($tags, 0, $tag_limit);
+
+    $tag_links = array();
+    foreach ($tags as $tag) {
+      $tag_links[] = phutil_render_tag(
+        'a',
+        array(
+          'href' => $request->generateURI(
+            array(
+              'action'  => 'browse',
+              'commit'  => $tag->getName(),
+            )),
+        ),
+        phutil_escape_html($tag->getName()));
+    }
+
+    if ($has_more_tags) {
+      $tag_links[] = phutil_render_tag(
+        'a',
+        array(
+          'href' => $request->generateURI(
+            array(
+              'action'  => 'tags',
+            )),
+        ),
+        "More tags\xE2\x80\xA6");
+    }
+
+    $tag_links = implode(', ', $tag_links);
+
+    return $tag_links;
+  }
 
 }

src/applications/diffusion/controller/commit/__init__.php

@@ -18,6 +18,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/committags/base');
 phutil_require_module('phabricator', 'applications/diffusion/query/contains/base');
 phutil_require_module('phabricator', 'applications/diffusion/query/mergedcommits/base');
 phutil_require_module('phabricator', 'applications/diffusion/query/parents/base');

src/applications/diffusion/controller/taglist/DiffusionTagListController.php

@@ -29,17 +29,33 @@ final class DiffusionTagListController extends DiffusionController {
     $pager->setURI($request->getRequestURI(), 'offset');
     $pager->setOffset($request->getInt('offset'));
 
-    $query = DiffusionTagListQuery::newFromDiffusionRequest($drequest);
-    $query->setOffset($pager->getOffset());
-    $query->setLimit($pager->getPageSize() + 1);
-    $tags = $query->loadTags();
+    if ($drequest->getRawCommit()) {
+      $is_commit = true;
+
+      $query = DiffusionCommitTagsQuery::newFromDiffusionRequest($drequest);
+      $query->setOffset($pager->getOffset());
+      $query->setLimit($pager->getPageSize() + 1);
+      $tags = $query->loadTags();
+    } else {
+      $is_commit = false;
+
+      $query = DiffusionTagListQuery::newFromDiffusionRequest($drequest);
+      $query->setOffset($pager->getOffset());
+      $query->setLimit($pager->getPageSize() + 1);
+      $tags = $query->loadTags();
+    }
+
     $tags = $pager->sliceResults($tags);
 
     $content = null;
     if (!$tags) {
       $content = new AphrontErrorView();
       $content->setTitle('No Tags');
-      $content->appendChild('This repository has no tags.');
+      if ($is_commit) {
+        $content->appendChild('This commit has no tags.');
+      } else {
+        $content->appendChild('This repository has no tags.');
+      }
       $content->setSeverity(AphrontErrorView::SEVERITY_NODATA);
     } else {
       $commits = id(new PhabricatorAuditCommitQuery())
@@ -69,7 +85,11 @@ final class DiffusionTagListController extends DiffusionController {
 
     return $this->buildStandardPageResponse(
       array(
-        $this->buildCrumbs(array('tags' => true)),
+        $this->buildCrumbs(
+          array(
+            'tags'    => true,
+            'commit'  => $drequest->getRawCommit(),
+          )),
         $content,
       ),
       array(

src/applications/diffusion/controller/taglist/__init__.php

@@ -8,6 +8,7 @@
 
 phutil_require_module('phabricator', 'applications/audit/query/commit');
 phutil_require_module('phabricator', 'applications/diffusion/controller/base');
+phutil_require_module('phabricator', 'applications/diffusion/query/committags/base');
 phutil_require_module('phabricator', 'applications/diffusion/query/taglist/base');
 phutil_require_module('phabricator', 'applications/diffusion/view/taglist');
 phutil_require_module('phabricator', 'applications/phid/handle/data');

src/applications/diffusion/query/committags/base/DiffusionCommitTagsQuery.php

@@ -0,0 +1,51 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+abstract class DiffusionCommitTagsQuery extends DiffusionQuery {
+
+  private $limit;
+  private $offset;
+
+  public function setOffset($offset) {
+    $this->offset = $offset;
+    return $this;
+  }
+
+  public function getOffset() {
+    return $this->offset;
+  }
+
+  public function setLimit($limit) {
+    $this->limit = $limit;
+    return $this;
+  }
+
+  protected function getLimit() {
+    return $this->limit;
+  }
+
+  final public static function newFromDiffusionRequest(
+    DiffusionRequest $request) {
+    return self::newQueryObject(__CLASS__, $request);
+  }
+
+  final public function loadTags() {
+    return $this->executeQuery();
+  }
+
+}

src/applications/diffusion/query/committags/base/__init__.php

@@ -0,0 +1,12 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/diffusion/query/base');
+
+
+phutil_require_source('DiffusionCommitTagsQuery.php');

src/applications/diffusion/query/committags/git/DiffusionGitCommitTagsQuery.php

@@ -0,0 +1,64 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class DiffusionGitCommitTagsQuery
+  extends DiffusionCommitTagsQuery {
+
+  protected function executeQuery() {
+    $drequest = $this->getRequest();
+    $repository = $drequest->getRepository();
+
+    list($err, $stdout) = $repository->execLocalCommand(
+      'tag -l --contains %s',
+      $drequest->getCommit());
+
+    if ($err) {
+      // Git exits with an error code if the commit is bogus.
+      return array();
+    }
+
+    $stdout = trim($stdout);
+    if (!strlen($stdout)) {
+      return array();
+    }
+
+    $tag_names = explode("\n", $stdout);
+    $tag_names = array_fill_keys($tag_names, true);
+
+    $tag_query = DiffusionTagListQuery::newFromDiffusionRequest($drequest);
+    $tags = $tag_query->loadTags();
+
+    $result = array();
+    foreach ($tags as $tag) {
+      if (isset($tag_names[$tag->getName()])) {
+        $result[] = $tag;
+      }
+    }
+
+    if ($this->getOffset()) {
+      $result = array_slice($result, $this->getOffset());
+    }
+
+    if ($this->getLimit()) {
+      $result = array_slice($result, 0, $this->getLimit());
+    }
+
+    return $result;
+  }
+
+}

src/applications/diffusion/query/committags/git/__init__.php

@@ -0,0 +1,13 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/diffusion/query/committags/base');
+phutil_require_module('phabricator', 'applications/diffusion/query/taglist/base');
+
+
+phutil_require_source('DiffusionGitCommitTagsQuery.php');

src/applications/diffusion/query/committags/mercurial/DiffusionMercurialCommitTagsQuery.php

@@ -0,0 +1,27 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class DiffusionMercurialCommitTagsQuery
+  extends DiffusionCommitTagsQuery {
+
+  protected function executeQuery() {
+    // TODO: Implement this.
+    return array();
+  }
+
+}

src/applications/diffusion/query/committags/mercurial/__init__.php

@@ -0,0 +1,12 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/diffusion/query/committags/base');
+
+
+phutil_require_source('DiffusionMercurialCommitTagsQuery.php');

src/applications/diffusion/query/committags/svn/DiffusionSvnCommitTagsQuery.php

@@ -0,0 +1,27 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class DiffusionSvnCommitTagsQuery
+  extends DiffusionCommitTagsQuery {
+
+  protected function executeQuery() {
+    // No meaningful concept of tags in Subversion.
+    return array();
+  }
+
+}

src/applications/diffusion/query/committags/svn/__init__.php

@@ -0,0 +1,12 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/diffusion/query/committags/base');
+
+
+phutil_require_source('DiffusionSvnCommitTagsQuery.php');

src/applications/diffusion/request/base/DiffusionRequest.php

@@ -31,8 +31,11 @@ abstract class DiffusionRequest {
   protected $callsign;
   protected $path;
   protected $line;
+  protected $symbolicCommit;
   protected $commit;
   protected $branch;
+  protected $commitType = 'commit';
+  protected $tagContent;
 
   protected $repository;
   protected $repositoryCommit;
@@ -174,9 +177,10 @@ abstract class DiffusionRequest {
    * @task new
    */
   final private function initializeFromDictionary(array $data) {
-    $this->path       = idx($data, 'path');
-    $this->commit     = idx($data, 'commit');
-    $this->line       = idx($data, 'line');
+    $this->path           = idx($data, 'path');
+    $this->symbolicCommit = idx($data, 'commit');
+    $this->commit         = idx($data, 'commit');
+    $this->line           = idx($data, 'line');
 
     if ($this->getSupportsBranches()) {
       $this->branch = idx($data, 'branch');
@@ -206,10 +210,18 @@ abstract class DiffusionRequest {
     return $this->commit;
   }
 
+  public function getSymbolicCommit() {
+    return $this->symbolicCommit;
+  }
+
   public function getBranch() {
     return $this->branch;
   }
 
+  public function getTagContent() {
+    return $this->tagContent;
+  }
+
   public function loadCommit() {
     if (empty($this->repositoryCommit)) {
       $repository = $this->getRepository();

src/applications/diffusion/request/git/DiffusionGitRequest.php

@@ -30,14 +30,46 @@ final class DiffusionGitRequest extends DiffusionRequest {
       return;
     }
 
-    // Expand commit short forms to full 40-character hashes. This does not
-    // verify them, --verify exits with return code 0 for anything that
-    // looks like a valid hash.
+    // Expand short commit names and verify
 
-    list($commit) = $this->getRepository()->execxLocalCommand(
-      'rev-parse --verify %s',
-      $this->commit);
-    $this->commit = trim($commit);
+    $future = $this->getRepository()->getLocalCommandFuture(
+      'cat-file --batch');
+    $future->write($this->commit);
+    list($stdout) = $future->resolvex();
+
+    list($hash, $type) = explode(' ', $stdout);
+    if ($type == 'missing') {
+      throw new Exception("Bad commit '{$this->commit}'.");
+    }
+
+    switch ($type) {
+      case 'tag':
+        $this->commitType = 'tag';
+
+        $matches = null;
+        $ok = preg_match(
+          '/^object ([a-f0-9]+)$.*?\n\n(.*)$/sm',
+          $stdout,
+          $matches);
+        if (!$ok) {
+          throw new Exception(
+            "Unparseable output from cat-file: {$stdout}");
+        }
+
+        $hash = $matches[1];
+        $this->tagContent = trim($matches[2]);
+        break;
+      case 'commit':
+        break;
+      default:
+        throw new AphrontUsageException(
+          "Invalid Object Name",
+          "The reference '{$this->commit}' does not name a valid ".
+          "commit or a tag in this repository.");
+        break;
+    }
+
+    $this->commit = $hash;
   }
 
   public function getBranch() {

src/applications/diffusion/request/git/__init__.php

@@ -6,6 +6,7 @@
 
 
 
+phutil_require_module('phabricator', 'aphront/exception/usage');
 phutil_require_module('phabricator', 'applications/diffusion/data/branch');
 phutil_require_module('phabricator', 'applications/diffusion/request/base');