Prefill template update message in Mercurial during --update

Summary:
When a user invokes the update flow from Mercurial, prefill the update message
like we do for Git.

Also add a little caching to improve performance since "hg" execs cost ~100ms.

Test Plan: Updated a Mercurial diff repeatedly, got sensible default message
fills.

Reviewers: Makinde, btrahan

Reviewed By: Makinde

CC: aran, epriestley

Maniphest Tasks: T28

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

src/repository/api/mercurial/ArcanistMercurialAPI.php

@@ -26,6 +26,8 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   private $status;
   private $base;
   private $relativeCommit;
+  private $workingCopyRevision;
+  private $localCommitInfo;
 
   public function execxLocal($pattern /*, ... */) {
     $args = func_get_args();
@@ -140,37 +142,40 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   }
 
   public function getLocalCommitInformation() {
-    list($info) = $this->execxLocal(
+    if ($this->localCommitInfo === null) {
-      'log --style default --rev %s..%s --',
+      list($info) = $this->execxLocal(
-      $this->getRelativeCommit(),
+        'log --style default --rev %s..%s --',
-      $this->getWorkingCopyRevision());
+        $this->getRelativeCommit(),
-    $logs = ArcanistMercurialParser::parseMercurialLog($info);
+        $this->getWorkingCopyRevision());
+      $logs = ArcanistMercurialParser::parseMercurialLog($info);
 
-    // Get rid of the first log, it's not actually part of the diff. "hg log"
+      // Get rid of the first log, it's not actually part of the diff. "hg log"
-    // is inclusive, while "hg diff" is exclusive.
+      // is inclusive, while "hg diff" is exclusive.
-    array_shift($logs);
+      array_shift($logs);
 
-    // Expand short hashes (12 characters) to full hashes (40 characters) by
+      // Expand short hashes (12 characters) to full hashes (40 characters) by
-    // issuing a big "hg log" command. Possibly we should do this with parents
+      // issuing a big "hg log" command. Possibly we should do this with parents
-    // too, but nothing uses them directly at the moment.
+      // too, but nothing uses them directly at the moment.
-    if ($logs) {
+      if ($logs) {
-      $cmd = array();
+        $cmd = array();
-      foreach (ipull($logs, 'rev') as $rev) {
+        foreach (ipull($logs, 'rev') as $rev) {
-        $cmd[] = csprintf('--rev %s', $rev);
+          $cmd[] = csprintf('--rev %s', $rev);
-      }
+        }
-
+
-      list($full) = $this->execxLocal(
+        list($full) = $this->execxLocal(
-        'log --template %s %C --',
+          'log --template %s %C --',
-        '{node}\\n',
+          '{node}\\n',
-        implode(' ', $cmd));
+          implode(' ', $cmd));
-
+
-      $full = explode("\n", trim($full));
+        $full = explode("\n", trim($full));
-      foreach ($logs as $key => $dict) {
+        foreach ($logs as $key => $dict) {
-        $logs[$key]['rev'] = array_pop($full);
+          $logs[$key]['rev'] = array_pop($full);
+        }
       }
+      $this->localCommitInfo = $logs;
     }
 
-    return $logs;
+    return $this->localCommitInfo;
   }
 
   public function getBlame($path) {
@@ -318,20 +323,25 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
   }
 
   public function getWorkingCopyRevision() {
-    // In Mercurial, "tip" means the tip of the current branch, not what's in
+    if ($this->workingCopyRevision === null) {
-    // the working copy. The tip may be ahead of the working copy. We need to
+      // In Mercurial, "tip" means the tip of the current branch, not what's in
-    // use "hg summary" to figure out what is actually in the working copy.
+      // the working copy. The tip may be ahead of the working copy. We need to
-    // For instance, "hg up 4 && arc diff" should not show commits 5 and above.
+      // use "hg summary" to figure out what is actually in the working copy.
+      // For instance, "hg up 4 && arc diff" should not show commits 5 and
+      // above.
 
-    // Without arguments, "hg id" shows the current working directory's commit,
+      // Without arguments, "hg id" shows the current working directory's
-    // and "--debug" expands it to a 40-character hash.
+      // commit, and "--debug" expands it to a 40-character hash.
-    list($stdout) = $this->execxLocal('--debug id --id');
+      list($stdout) = $this->execxLocal('--debug id --id');
 
-    // Even with "--id", "hg id" will print a trailing "+" after the hash
+      // Even with "--id", "hg id" will print a trailing "+" after the hash
-    // if the working copy is dirty (has uncommitted changes). We'll explicitly
+      // if the working copy is dirty (has uncommitted changes). We'll
-    // detect this later by calling getWorkingCopyStatus(); ignore it for now.
+      // explicitly detect this later by calling getWorkingCopyStatus(); ignore
-    $stdout = trim($stdout);
+      // it for now.
-    return rtrim($stdout, '+');
+      $stdout = trim($stdout);
+      $this->workingCopyRevision = rtrim($stdout, '+');
+    }
+    return $this->workingCopyRevision;
   }
 
   public function supportsRelativeLocalCommits() {
@@ -384,6 +394,23 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
            "'hg push' or by printing and faxing it).";
   }
 
+  public function getCommitMessageLog() {
+    list($stdout) = $this->execxLocal(
+      "log --template '{node}\\1{desc}\\0' --rev %s..%s --",
+      $this->getRelativeCommit(),
+      $this->getWorkingCopyRevision());
+
+    $map = array();
+
+    $logs = explode("\0", trim($stdout));
+    foreach (array_filter($logs) as $log) {
+      list($node, $desc) = explode("\1", $log);
+      $map[$node] = $desc;
+    }
+
+    return array_reverse($map);
+  }
+
   public function loadWorkingCopyDifferentialRevisions(
     ConduitClient $conduit,
     array $query) {
@@ -400,17 +427,6 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
         'commitHashes' => $hashes,
       ));
 
-    if ($results) {
-      return $results;
-    }
-
-    // If we still didn't succeed, try to find revisions by branch name.
-    $results = $conduit->callMethodSynchronous(
-      'differential.query',
-      $query + array(
-        'branches' => array($this->getBranchName()),
-      ));
-
     return $results;
   }
 

src/workflow/diff/ArcanistDiffWorkflow.php

@@ -1590,11 +1590,15 @@ EOTEXT
       return $this->getGitUpdateMessage();
     }
 
+    if ($repository_api instanceof ArcanistMercurialAPI) {
+      return $this->getMercurialUpdateMessage();
+    }
+
     return null;
   }
 
   /**
-   * Retrieve the git message in HEAD if it isn't a primary template message.
+   * Retrieve the git messages between HEAD and the last update.
    *
    * @task message
    */
@@ -1617,14 +1621,7 @@ EOTEXT
     // we always get this 100% right, we're just trying to do something
     // reasonable.
 
-    $current_diff = $this->getConduit()->callMethodSynchronous(
+    $local = $this->loadActiveLocalCommitInfo();
-      'differential.getdiff',
-      array(
-        'revision_id' => $this->revisionID,
-      ));
-
-    $properties = idx($current_diff, 'properties', array());
-    $local = idx($properties, 'local:commits', array());
     $hashes = ipull($local, null, 'commit');
 
     $usable = array();
@@ -1653,6 +1650,49 @@ EOTEXT
       return null;
     }
 
+    return $this->formatUsableLogs($usable);
+  }
+
+  /**
+   * Retrieve the hg messages between tip and the last update.
+   *
+   * @task message
+   */
+  private function getMercurialUpdateMessage() {
+    $repository_api = $this->getRepositoryAPI();
+
+    $messages = $repository_api->getCommitMessageLog();
+
+    $local = $this->loadActiveLocalCommitInfo();
+    $hashes = ipull($local, null, 'rev');
+
+    $usable = array();
+    foreach ($messages as $rev => $message) {
+      if (isset($hashes[$rev])) {
+        // If this commit is currently part of the active diff on the revision,
+        // stop using commit messages, since anything older than this isn't new.
+        break;
+      }
+
+      // Otherwise, this looks new, so it's a usable commit message.
+      $usable[] = $message;
+    }
+
+    if (!$usable) {
+      // No new commit messages, so we don't have anywhere to start from.
+      return null;
+    }
+
+    return $this->formatUsableLogs($usable);
+  }
+
+
+  /**
+   * Format log messages to prefill a diff update.
+   *
+   * @task message
+   */
+  private function formatUsableLogs(array $usable) {
     // Flip messages so they'll read chronologically (oldest-first) in the
     // template, e.g.:
     //
@@ -1664,7 +1704,7 @@ EOTEXT
     $default = array();
     foreach ($usable as $message) {
       // Pick the first line out of each message.
-      $text = trim($message->getMetadata('message'));
+      $text = trim($message);
       $text = head(explode("\n", $text));
       $default[] = '  - '.$text."\n";
     }
@@ -1672,6 +1712,17 @@ EOTEXT
     return implode('', $default);
   }
 
+  private function loadActiveLocalCommitInfo() {
+    $current_diff = $this->getConduit()->callMethodSynchronous(
+      'differential.getdiff',
+      array(
+        'revision_id' => $this->revisionID,
+      ));
+
+    $properties = idx($current_diff, 'properties', array());
+    return idx($properties, 'local:commits', array());
+  }
+
 
 /* -(  Diff Specification  )------------------------------------------------- */