Make "arc which" really really really verbose

Summary: Provide far more information about what "arc diff" intends to do.

Test Plan: Ran "arc which" in a variety of circumstances.

Reviewers: btrahan, Makinde

Reviewed By: Makinde

CC: aran

Maniphest Tasks: T1183

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

src/repository/api/base/ArcanistRepositoryAPI.php

@@ -194,6 +194,14 @@ abstract class ArcanistRepositoryAPI {
     throw new ArcanistCapabilityNotSupportedException($this);
   }
 
+  public function getRelativeExplanation() {
+    throw new ArcanistCapabilityNotSupportedException($this);
+  }
+
+  public function getCommitSummary($commit) {
+    throw new ArcanistCapabilityNotSupportedException($this);
+  }
+
   public function getAllLocalChanges() {
     throw new ArcanistCapabilityNotSupportedException($this);
   }

src/repository/api/git/ArcanistGitAPI.php

@@ -25,6 +25,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
 
   private $status;
   private $relativeCommit = null;
+  private $relativeExplanation = '???';
   private $repositoryHasNoCommits = false;
   const SEARCH_LENGTH_FOR_PARENT_REVISIONS = 16;
 
@@ -93,6 +94,10 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
     $commits = array();
 
     $info = trim($info, " \n\2");
+    if (!strlen($info)) {
+      return array();
+    }
+
     $info = explode("\2", $info);
     foreach ($info as $line) {
       list($commit, $tree, $parents, $time, $author, $title, $message)
@@ -128,6 +133,12 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
 
         $this->relativeCommit = self::GIT_MAGIC_ROOT_COMMIT;
 
+        if ($this->repositoryHasNoCommits) {
+          $this->relativeExplanation = "the repository has no commits.";
+        } else {
+          $this->relativeExplanation = "the repository has only one commit.";
+        }
+
         return $this->relativeCommit;
       }
 
@@ -137,6 +148,10 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
       if ($working_copy) {
         $default_relative = $working_copy->getConfig(
           'git.default-relative-commit');
+        $this->relativeExplanation =
+          "it is the merge-base of '{$default_relative}' and HEAD, as ".
+          "specified in 'git.default-relative-commit' in '.arcconfig'. This ".
+          "setting overrides other settings.";
       }
 
       if (!$default_relative) {
@@ -145,12 +160,20 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
 
         if (!$err) {
           $default_relative = trim($upstream);
+          $this->relativeExplanation =
+            "it is the merge-base of '{$default_relative}' (the Git upstream ".
+            "of the current branch) HEAD.";
         }
       }
 
       if (!$default_relative) {
         $default_relative = $this->readScratchFile('default-relative-commit');
         $default_relative = trim($default_relative);
+        if ($default_relative) {
+          $this->relativeExplanation =
+            "it is the merge-base of '{$default_relative}' and HEAD, as ".
+            "specified in '.arc/default-relative-commit'.";
+        }
       }
 
       if (!$default_relative) {
@@ -200,6 +223,9 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
         // Don't perform this write until we've verified that the object is a
         // valid commit name.
         $this->writeScratchFile('default-relative-commit', $default_relative);
+        $this->relativeExplanation =
+          "it is the merge-base of '{$default_relative}' and HEAD, as you ".
+          "just specified.";
       }
 
       list($merge_base) = $this->execxLocal(
@@ -658,6 +684,8 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
     $base = reset($argv);
     if ($base == ArcanistGitAPI::GIT_MAGIC_ROOT_COMMIT) {
       $merge_base = $base;
+      $this->relativeExplanation =
+        "you explicitly specified the empty tree.";
     } else {
       list($err, $merge_base) = $this->execManualLocal(
         'merge-base %s HEAD',
@@ -666,6 +694,9 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
         throw new ArcanistUsageException(
           "Unable to find any git commit named '{$base}' in this repository.");
       }
+      $this->relativeExplanation =
+        "it is the merge-base of '{$base}' and HEAD, as you explicitly ".
+        "specified.";
     }
     $this->setRelativeCommit(trim($merge_base));
   }
@@ -725,12 +756,14 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
     $messages = $parser->parseDiff($messages);
 
     // First, try to find revisions by explicit revision IDs in commit messages.
+    $reason_map = array();
     $revision_ids = array();
     foreach ($messages as $message) {
       $object = ArcanistDifferentialCommitMessage::newFromRawCorpus(
         $message->getMetadata('message'));
       if ($object->getRevisionID()) {
         $revision_ids[] = $object->getRevisionID();
+        $reason_map[$object->getRevisionID()] = $message->getCommitHash();
       }
     }
 
@@ -740,6 +773,13 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
         $query + array(
           'ids' => $revision_ids,
         ));
+
+      foreach ($results as $key => $result) {
+        $hash = substr($reason_map[$result['id']], 0, 16);
+        $results[$key]['why'] =
+          "Commit message for '{$hash}' has explicit 'Differential Revision'.";
+      }
+
       return $results;
     }
 
@@ -756,6 +796,12 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
         'commitHashes' => $hashes,
       ));
 
+    foreach ($results as $key => $result) {
+      $results[$key]['why'] =
+        "A git commit or tree hash in the commit range is already attached ".
+        "to the Differential revision.";
+    }
+
     return $results;
   }
 
@@ -763,4 +809,21 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
     $this->execxLocal('pull');
   }
 
+  public function getRelativeExplanation() {
+    return $this->relativeExplanation;
+  }
+
+  public function getCommitSummary($commit) {
+    if ($commit == self::GIT_MAGIC_ROOT_COMMIT) {
+      return '(The Empty Tree)';
+    }
+
+    list($summary) = $this->execxLocal(
+      'log -n 1 --format=%C %s',
+      '%s',
+      $commit);
+
+    return trim($summary);
+  }
+
 }

src/repository/api/mercurial/ArcanistMercurialAPI.php

@@ -26,6 +26,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   private $status;
   private $base;
   private $relativeCommit;
+  private $relativeExplanation;
   private $workingCopyRevision;
   private $localCommitInfo;
   private $includeDirectoryStateInDiffs;
@@ -107,6 +108,10 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
       }
 
       if (!$logs) {
+
+        $this->relativeExplanation =
+          "you have no outgoing commits, so arc assumes you intend to submit ".
+          "uncommitted changes in the working copy.";
         // In Mercurial, we support operations against uncommitted changes.
         $this->setRelativeCommit($this->getWorkingCopyRevision());
         return $this->relativeCommit;
@@ -148,6 +153,15 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
         }
       }
 
+      if ($against == 'null') {
+        $this->relativeExplanation =
+          "this is a new repository (all changes are outgoing).";
+      } else {
+        $this->relativeExplanation =
+          "it is the first commit reachable from the working copy state ".
+          "which is not outgoing.";
+      }
+
       $this->setRelativeCommit($against);
     }
     return $this->relativeCommit;
@@ -163,8 +177,6 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
         $this->getWorkingCopyRevision());
       $logs = array_filter(explode("\2", $info));
 
-      array_shift($logs);
-
       $last_node = null;
       $is_first = true;
 
@@ -200,6 +212,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
           'branch'  => $branch,
           'tag'     => $tag,
           'commit'  => $node,
+          'rev'     => $node, // TODO: Remove eventually.
           'local'   => $rev,
           'parents' => $commit_parents,
           'summary' => head(explode("\n", $desc)),
@@ -415,6 +428,9 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
     if (count($argv) != 1) {
       throw new ArcanistUsageException("Specify only one commit.");
     }
+
+    $this->relativeExplanation = "you explicitly specified it.";
+
     // This does the "hg id" call we need to normalize/validate the revision
     // identifier.
     $this->setRelativeCommit(reset($argv));
@@ -481,7 +497,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
     // Try to find revisions by hash.
     $hashes = array();
     foreach ($this->getLocalCommitInformation() as $commit) {
-      $hashes[] = array('hgcm', $commit['rev']);
+      $hashes[] = array('hgcm', $commit['commit']);
     }
 
     $results = $conduit->callMethodSynchronous(
@@ -490,6 +506,12 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
         '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;
   }
 
@@ -519,4 +541,22 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
     $this->localCommitInfo = null;
   }
 
+  public function getRelativeExplanation() {
+    return $this->relativeExplanation;
+  }
+
+  public function getCommitSummary($commit) {
+    if ($commit == 'null') {
+      return '(The Empty Void)';
+    }
+
+    list($summary) = $this->execxLocal(
+      'log --template {desc} --limit 1 --rev %s',
+      $commit);
+
+    $summary = head(explode("\n", $summary));
+
+    return trim($summary);
+  }
+
 }

src/repository/api/subversion/ArcanistSubversionAPI.php

@@ -553,6 +553,11 @@ EODIFF;
       }
     }
 
+    foreach ($results as $key => $result) {
+      $results[$key]['why'] =
+        "Matching arcanist project name and working copy directory path.";
+    }
+
     return $results;
   }
 

src/workflow/which/ArcanistWhichWorkflow.php

@@ -34,8 +34,8 @@ EOTEXT
   public function getCommandHelp() {
     return phutil_console_format(<<<EOTEXT
           Supports: svn, git, hg
-          Shows which revision is in the working copy (or which revisions, if
+          Shows which commits 'arc diff' will select, and which revision is in
-          more than one matches).
+          the working copy (or which revisions, if more than one matches).
 EOTEXT
       );
   }
@@ -60,10 +60,6 @@ EOTEXT
       'any-status' => array(
         'help' => "Show committed and abandoned revisions.",
       ),
-      'id' => array(
-        'help' => "If exactly one revision matches, print it to stdout. ".
-                  "Otherwise, exit with an error. Intended for scripts.",
-      ),
       '*' => 'commit',
     );
   }
@@ -72,15 +68,63 @@ EOTEXT
 
     $repository_api = $this->getRepositoryAPI();
 
-    $commit = $this->getArgument('commit');
+    $arg_commit = $this->getArgument('commit');
-    if (count($commit)) {
+    if (count($arg_commit)) {
       if (!$repository_api->supportsRelativeLocalCommits()) {
         throw new ArcanistUsageException(
           "This version control system does not support relative commits.");
       } else {
-        $repository_api->parseRelativeLocalCommit($commit);
+        $repository_api->parseRelativeLocalCommit($arg_commit);
       }
     }
+    $arg = $arg_commit ? ' '.head($arg_commit) : '';
+
+    if ($repository_api->supportsRelativeLocalCommits()) {
+      $relative = $repository_api->getRelativeCommit();
+
+      $info = $repository_api->getLocalCommitInformation();
+      if ($info) {
+        $commits = array();
+        foreach ($info as $commit) {
+          $hash     = substr($commit['commit'], 0, 16);
+          $summary  = $commit['summary'];
+
+          $commits[] = "    {$hash}  {$summary}";
+        }
+        $commits = implode("\n", $commits);
+      } else {
+        $commits = '    (No commits.)';
+      }
+
+      $explanation = $repository_api->getRelativeExplanation();
+
+      $relative_summary = $repository_api->getCommitSummary($relative);
+      $relative = substr($relative, 0, 16);
+
+      if ($repository_api instanceof ArcanistGitAPI) {
+        $command = "git diff {$relative}..HEAD";
+      } else if ($repository_api instanceof ArcanistMercurialAPI) {
+        $command = "hg diff --rev {$relative} --rev .";
+      } else {
+        throw new Exception("Unknown VCS!");
+      }
+
+      echo phutil_console_wrap(
+        phutil_console_format(
+          "**RELATIVE COMMIT**\n".
+          "If you run 'arc diff{$arg}', changes between the commit:\n\n"));
+
+      echo  "    {$relative}  {$relative_summary}\n\n";
+      echo phutil_console_wrap(
+        "...and the current working copy state will be sent to ".
+        "Differential, because {$explanation}\n\n".
+        "You can see the exact changes that will be sent by running ".
+        "this command:\n\n".
+        "    $ {$command}\n\n".
+        "These commits will be included in the diff:\n\n");
+
+      echo $commits."\n\n\n";
+    }
 
     $any_author = $this->getArgument('any-author');
     $any_status = $this->getArgument('any-status');
@@ -98,23 +142,38 @@ EOTEXT
       $this->getConduit(),
       $query);
 
+    echo phutil_console_wrap(
+      phutil_console_format(
+        "**MATCHING REVISIONS**\n".
+        "These Differential revisions match the changes in this working ".
+        "copy:\n\n"));
+
     if (empty($revisions)) {
-      $this->writeStatusMessage("No matching revisions.\n");
+      echo "    (No revisions match.)\n";
-      return 1;
+      echo "\n";
-    }
+      echo phutil_console_wrap(
-
+        phutil_console_format(
-    if ($this->getArgument('id')) {
+          "Since there are no revisions in Differential which match this ".
-      if (count($revisions) == 1) {
+          "working copy, a new revision will be **created** if you run ".
-        echo idx(head($revisions), 'id');
+          "'arc diff{$arg}'.\n\n"));
-        return 0;
+    } else {
-      } else {
+      foreach ($revisions as $revision) {
-        $this->writeStatusMessage("More than one matching revision.\n");
+        echo '    D'.$revision['id'].' '.$revision['title']."\n";
-        return 1;
+        echo '        Reason: '.$revision['why']."\n";
+        echo "\n";
       }
-    }
+      if (count($revisions) == 1) {
-
+        echo phutil_console_wrap(
-    foreach ($revisions as $revision) {
+          phutil_console_format(
-      echo 'D'.$revision['id'].' '.$revision['title']."\n";
+            "Since exactly one revision in Differential matches this working ".
+            "copy, it will be **updated** if you run 'arc diff{$arg}'."));
+      } else {
+        echo phutil_console_wrap(
+          "Since more than one revision in Differential matches this working ".
+          "copy, you will be asked which revision you want to update if ".
+          "you run 'arc diff {$arg}'.");
+      }
+      echo "\n\n";
     }
 
     return 0;