fbamend: show a hint when split commits have the same Phabricator Diff

Reviewed By: quark-zju

Differential Revision: D10105739

fbshipit-source-id: 7f2c7664a2e8eb4704ff2e5800c1c24cbedb3fdc

hgext/fbamend/__init__.py

@@ -44,6 +44,12 @@ conflicts. To never automatically restack::
 
 Note that if --date is specified on the command line, it takes precedence.
 
+If a split creates multiple commits that have the same phabricator diff, the
+following advice for resolution will be shown::
+
+    [split]
+    phabricatoradvice = edit the commit messages to remove the association
+
 """
 
 from __future__ import absolute_import
@@ -171,6 +177,14 @@ def hintprev():
     return _("use 'hg prev' to move to the parent changeset")
 
 
+@hint("split-phabricator")
+def hintsplitphabricator(advice):
+    msg = _("some split commits have the same Phabricator Diff associated with them")
+    if advice:
+        msg += "\n" + advice
+    return msg
+
+
 def uisetup(ui):
     hiddenoverride.uisetup(ui)
     prune.uisetup(ui)

hgext/fbamend/split.py

@@ -17,6 +17,7 @@ from mercurial import (
     commands,
     error,
     hg,
+    hintutil,
     lock as lockmod,
     obsolete,
     registrar,
@@ -25,6 +26,7 @@ from mercurial import (
 from mercurial.i18n import _
 
 from . import common
+from ..extlib.phabricator import diffprops
 
 
 cmdtable = {}
@@ -128,6 +130,16 @@ def split(ui, repo, *revs, **opts):
                 ui.status(_("no more change to split\n"))
 
         if newcommits:
+            phabdiffs = {}
+            for c in newcommits:
+                phabdiff = diffprops.parserevfromcommitmsg(repo[c].description())
+                if phabdiff:
+                    phabdiffs.setdefault(phabdiff, []).append(c)
+            if any(len(commits) > 1 for commits in phabdiffs.values()):
+                hintutil.trigger(
+                    "split-phabricator", ui.config("split", "phabricatoradvice")
+                )
+
             tip = repo[newcommits[-1]]
             bmupdate(tip.node())
             if bookactive is not None:

tests/test-fb-hgext-fbamend-split.t

@@ -11,6 +11,8 @@ Set up test environment.
   > interactive = true
   > [fbamend]
   > safestrip = false
+  > [hint]
+  > ack-hint-ack = true
   > EOF
   $ mkcommit() {
   >    echo "${1}1" > "${1}1"
@@ -347,3 +349,38 @@ Test that bookmarks are correctly moved.
   $ hg bookmarks
      test1                     2:* (glob)
    * test2                     2:* (glob)
+
+Test the hint for Phabricator Diffs being duplicated
+  $ cd ..
+  $ newrepo
+  $ echo 1 > a1
+  $ echo 2 > a2
+  $ hg commit -Aqm "Differential Revision: http://example.com/D1234"
+  $ hg split --config split.phabricatoradvice="amend the commit messages to remove them" << EOF
+  > y
+  > y
+  > n
+  > y
+  > EOF
+  0 files updated, 0 files merged, 2 files removed, 0 files unresolved
+  adding a1
+  adding a2
+  diff --git a/a1 b/a1
+  new file mode 100644
+  examine changes to 'a1'? [Ynesfdaq?] y
+  
+  @@ -0,0 +1,1 @@
+  +1
+  record change 1/2 to 'a1'? [Ynesfdaq?] y
+  
+  diff --git a/a2 b/a2
+  new file mode 100644
+  examine changes to 'a2'? [Ynesfdaq?] n
+  
+  Done splitting? [yN] y
+  hint[split-phabricator]: some split commits have the same Phabricator Diff associated with them
+  amend the commit messages to remove them
+  $ showgraph
+  @  2 b696183283c3 Differential Revision: http://example.com/D1234
+  |
+  o  1 6add538b4b79 Differential Revision: http://example.com/D1234