From d3436c256cbaf3a3b6867b570dc89e0d789a80ac Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 27 Apr 2015 03:51:21 -0700
Subject: [PATCH] Ignore closed branch heads by default in Mercurial

Summary:
Fixes T6160. Ref T7100.

  - When resolving ambiguous branch references, ignore closed heads unless there are no other options.
  - Hide closed heads by default on the main page.
  - Show branch open/closed state in Mercurial.

Test Plan: Browsed a previously-ambiguous Mercurial repository because of multiple branch heads, no longer ambiguous.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6160, T7100

Differential Revision: https://secure.phabricator.com/D12552
---
 .../DiffusionBranchQueryConduitAPIMethod.php  | 11 ++++++++
 .../DiffusionRepositoryController.php         |  1 +
 .../diffusion/request/DiffusionRequest.php    | 26 ++++++++++++++++---
 .../view/DiffusionBranchTableView.php         | 20 ++++++++++++++
 4 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php
index 1271088b1b..5fd440e4b2 100644
--- a/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php
+++ b/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php
@@ -17,6 +17,7 @@ final class DiffusionBranchQueryConduitAPIMethod
 
   protected function defineCustomParamTypes() {
     return array(
+      'closed' => 'optional bool',
       'limit' => 'optional int',
       'offset' => 'optional int',
       'contains' => 'optional string',
@@ -97,6 +98,16 @@ final class DiffusionBranchQueryConduitAPIMethod
       }
     }
 
+    $with_closed = $request->getValue('closed');
+    if ($with_closed !== null) {
+      foreach ($refs as $key => $ref) {
+        $fields = $ref->getRawFields();
+        if (idx($fields, 'closed') != $with_closed) {
+          unset($refs[$key]);
+        }
+      }
+    }
+
     // NOTE: We can't apply the offset or limit until here, because we may have
     // filtered untrackable branches out of the result set.
 
diff --git a/src/applications/diffusion/controller/DiffusionRepositoryController.php b/src/applications/diffusion/controller/DiffusionRepositoryController.php
index 46707f8430..7eb99b7096 100644
--- a/src/applications/diffusion/controller/DiffusionRepositoryController.php
+++ b/src/applications/diffusion/controller/DiffusionRepositoryController.php
@@ -351,6 +351,7 @@ final class DiffusionRepositoryController extends DiffusionController {
     $branches = $this->callConduitWithDiffusionRequest(
       'diffusion.branchquery',
       array(
+        'closed' => false,
         'limit' => $limit + 1,
       ));
     if (!$branches) {
diff --git a/src/applications/diffusion/request/DiffusionRequest.php b/src/applications/diffusion/request/DiffusionRequest.php
index 9ffd67ac07..0bd386d7ff 100644
--- a/src/applications/diffusion/request/DiffusionRequest.php
+++ b/src/applications/diffusion/request/DiffusionRequest.php
@@ -756,12 +756,32 @@ abstract class DiffusionRequest {
   }
 
   private function chooseBestRefMatch($ref, array $results) {
-    // TODO: Do a better job of selecting the best match.
-    $match = head($results);
+    // First, filter out less-desirable matches.
+    $candidates = array();
+    foreach ($results as $result) {
+      // Exclude closed heads.
+      if ($result['type'] == 'branch') {
+        if (idx($result, 'closed')) {
+          continue;
+        }
+      }
+
+      $candidates[] = $result;
+    }
+
+    // If we filtered everything, undo the filtering.
+    if (!$candidates) {
+      $candidates = $results;
+    }
+
+    // TODO: Do a better job of selecting the best match?
+    $match = head($candidates);
 
     // After choosing the best alternative, save all the alternatives so the
     // UI can show them to the user.
-    $this->refAlternatives = $results;
+    if (count($candidates) > 1) {
+      $this->refAlternatives = $candidates;
+    }
 
     return $match;
   }
diff --git a/src/applications/diffusion/view/DiffusionBranchTableView.php b/src/applications/diffusion/view/DiffusionBranchTableView.php
index b1e14688f1..d053d5bb33 100644
--- a/src/applications/diffusion/view/DiffusionBranchTableView.php
+++ b/src/applications/diffusion/view/DiffusionBranchTableView.php
@@ -22,6 +22,8 @@ final class DiffusionBranchTableView extends DiffusionView {
     $current_branch = $drequest->getBranch();
     $repository = $drequest->getRepository();
 
+    $can_close_branches = ($repository->isHg());
+
     Javelin::initBehavior('phabricator-tooltips');
 
     $doc_href = PhabricatorEnv::getDoclink('Diffusion User Guide: Autoclose');
@@ -75,6 +77,14 @@ final class DiffusionBranchTableView extends DiffusionView {
             'size' => 200,
           ));
 
+      $fields = $branch->getRawFields();
+      $closed = idx($fields, 'closed');
+      if ($closed) {
+        $status = pht('Closed');
+      } else {
+        $status = pht('Open');
+      }
+
       $rows[] = array(
         phutil_tag(
           'a',
@@ -99,6 +109,7 @@ final class DiffusionBranchTableView extends DiffusionView {
         self::linkCommit(
           $drequest->getRepository(),
           $branch->getCommitIdentifier()),
+        $status,
         $status_icon,
         $datetime,
         AphrontTableView::renderSingleDisplayLine($details),
@@ -116,6 +127,7 @@ final class DiffusionBranchTableView extends DiffusionView {
         pht('History'),
         pht('Branch'),
         pht('Head'),
+        pht('State'),
         pht(''),
         pht('Modified'),
         pht('Details'),
@@ -127,8 +139,16 @@ final class DiffusionBranchTableView extends DiffusionView {
         '',
         '',
         '',
+        '',
         'wide',
       ));
+    $view->setColumnVisibility(
+      array(
+        true,
+        true,
+        true,
+        $can_close_branches,
+      ));
     $view->setRowClasses($rowc);
     return $view->render();
   }