From 802c0ed89fe4eec8354384f283b35b29b5386761 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 17 Jul 2012 16:16:11 -0700
Subject: [PATCH] Fix some arc/mercurial issues

Summary:
  - In "arc which", we recommend "--rev x --rev ." to show changes. This is not accurate if there are uncommitted changes in the working copy. Just "--rev x" shows the correct changes (implicitly, the other end of the range is the working copy state).
  - When you diff only working copy changes, we currently incorrectly identify all your open revisions as belonging to the working copy. Instead, correctly identify none of them as belonging to the working copy (in theory, we could go farther than this and do path-based identification like SVN, but with --amend in hg 2.2+ this workflow should be going away in the long run).
  - If you have uncommitted working copy changes, never try to amend.

Test Plan: Ran "arc which .", "arc diff ." in a working copy with dirty changes, got better results than before.

Reviewers: dschleimer, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1507

Differential Revision: https://secure.phabricator.com/D2980
---
 src/repository/api/ArcanistMercurialAPI.php | 32 +++++++++++++--------
 src/workflow/ArcanistDiffWorkflow.php       | 25 +++++++++++++---
 src/workflow/ArcanistWhichWorkflow.php      |  2 +-
 3 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/src/repository/api/ArcanistMercurialAPI.php b/src/repository/api/ArcanistMercurialAPI.php
index e0f3aef3..cd22344b 100644
--- a/src/repository/api/ArcanistMercurialAPI.php
+++ b/src/repository/api/ArcanistMercurialAPI.php
@@ -417,9 +417,9 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   public function supportsAmend() {
     list ($err, $stdout) = $this->execManualLocal('help commit');
     if ($err) {
-        return false;
+      return false;
     } else {
-        return (strstr($stdout, "amend") !== false);
+      return (strstr($stdout, "amend") !== false);
     }
   }
 
@@ -553,19 +553,27 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
       $hashes[] = array('hgcm', $commit['commit']);
     }
 
-    $results = $conduit->callMethodSynchronous(
-      'differential.query',
-      $query + array(
-        'commitHashes' => $hashes,
-      ));
+    if ($hashes) {
 
-    foreach ($results as $key => $hash) {
-      $results[$key]['why'] =
-        "A mercurial commit hash in the commit range is already attached ".
-        "to the Differential revision.";
+      // NOTE: In the case of "arc diff . --uncommitted" in a Mercurial working
+      // copy with dirty changes, there may be no local commits.
+
+      $results = $conduit->callMethodSynchronous(
+        'differential.query',
+        $query + array(
+          'commitHashes' => $hashes,
+        ));
+
+      foreach ($results as $key => $hash) {
+        $results[$key]['why'] =
+          "A mercurial commit hash in the commit range is already attached ".
+          "to the Differential revision.";
+      }
+
+      return $results;
     }
 
-    return $results;
+    return array();
   }
 
   public function updateWorkingCopy() {
diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php
index 73c36f23..f45548f0 100644
--- a/src/workflow/ArcanistDiffWorkflow.php
+++ b/src/workflow/ArcanistDiffWorkflow.php
@@ -38,6 +38,7 @@ final class ArcanistDiffWorkflow extends ArcanistBaseWorkflow {
   private $unitWorkflow;
   private $lintWorkflow;
   private $postponedLinters;
+  private $haveUncommittedChanges = false;
 
   public function getCommandSynopses() {
     return phutil_console_format(<<<EOTEXT
@@ -297,7 +298,7 @@ EOTEXT
         'help' => 'Never amend commits in the working copy.',
       ),
       'uncommitted' => array(
-        'help' => 'Include uncommitted changes without prompting.',
+        'help' => 'Suppress warning about uncommitted changes.',
         'supports' => array(
           'hg',
         ),
@@ -486,9 +487,8 @@ EOTEXT
             'revision_id' => $result['revisionid'],
           ));
 
-        if (!$this->isRawDiffSource() && $this->shouldAmend()) {
+        if ($this->shouldAmend()) {
           $repository_api = $this->getRepositoryAPI();
-
           if ($repository_api->supportsAmend()) {
             echo "Updating commit message...\n";
             $repository_api->amendCommit($revised_message);
@@ -577,6 +577,7 @@ EOTEXT
           }
 
           $repository_api->setIncludeDirectoryStateInDiffs(true);
+          $this->haveUncommittedChanges = true;
         }
       }
     }
@@ -1070,7 +1071,23 @@ EOTEXT
   }
 
   private function shouldAmend() {
-    return !$this->isHistoryImmutable() && !$this->getArgument('no-amend');
+    if ($this->haveUncommittedChanges) {
+      return false;
+    }
+
+    if ($this->isHistoryImmutable()) {
+      return false;
+    }
+
+    if ($this->getArgument('no-amend')) {
+      return false;
+    }
+
+    if ($this->isRawDiffSource()) {
+      return false;
+    }
+
+    return true;
   }
 
 
diff --git a/src/workflow/ArcanistWhichWorkflow.php b/src/workflow/ArcanistWhichWorkflow.php
index 7178320e..e429fe81 100644
--- a/src/workflow/ArcanistWhichWorkflow.php
+++ b/src/workflow/ArcanistWhichWorkflow.php
@@ -127,7 +127,7 @@ EOTEXT
       if ($repository_api instanceof ArcanistGitAPI) {
         $command = "git diff {$relative}..HEAD";
       } else if ($repository_api instanceof ArcanistMercurialAPI) {
-        $command = "hg diff --rev {$relative} --rev .";
+        $command = "hg diff --rev {$relative}";
       } else {
         throw new Exception("Unknown VCS!");
       }