[PATCH 2 of 2 V2] split: improve commit message prompt

Jun Wu quark at fb.com
Sun Jun 25 02:08:01 EDT 2017


# HG changeset patch
# User Jun Wu <quark at fb.com>
# Date 1498368118 25200
#      Sat Jun 24 22:21:58 2017 -0700
# Node ID 85c2b9290440fe639029db10ab9e165e09ec0f2d
# Parent  0a25f04006ce0e4d50881fe6a053e1cbc18882ef
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 85c2b9290440
split: improve commit message prompt

One UX problem of split is what commit message to prompt. Previously, split
just prompt the commit message of the original changeset over and over.

That's not too friendly. The user may end up deleting same paragraphs over
and over, or leave duplicated commit messages in the repo. It's especially
problematic where commit message matters, like in a Phabricator setup,
the "Differential Revision" line should be unique per visible changeset.

This patch detects used paragraphs and remove them from the next prompt.
In case the user still want to have a look at them, "HG: " lines are added
to give the user context about committed split changesets.

diff --git a/hgext/split.py b/hgext/split.py
--- a/hgext/split.py
+++ b/hgext/split.py
@@ -19,4 +19,5 @@ from mercurial import (
     extensions,
     hg,
+    mdiff,
     node,
     obsolete,
@@ -134,15 +135,25 @@ def dosplit(ui, repo, tr, ctx, opts):
     # Any modified, added, removed, deleted result means split is incomplete
     incomplete = lambda repo: any(repo.status()[:4])
+    desc = initialdescription(ctx)
 
     # Main split loop
     while incomplete(repo):
+        hint = (_('This is the commit message for the #%d changeset split '
+                  'from:\n  %s: %s\n') % (len(committed) + 1, ctx,
+                                          ctx.description().split('\n', 1)[0]))
+        if committed:
+            hint += (_('Previous split commit messages are:\n\n%s')
+                     % '\n--\n'.join(indent(c.description())
+                                     for c in reversed(committed)))
+        message = '%s\n\n%s' % (desc, cmdutil.hgprefix(hint))
         opts.update({
             'edit': True,
             'interactive': True,
-            'message': ctx.description(),
+            'message': message,
         })
         commands.commit(ui, repo, **opts)
         newctx = repo['.']
         committed.append(newctx)
+        desc = nextdescription(desc, newctx)
 
     if not committed:
@@ -181,2 +192,35 @@ def dostrip(ui, repo, node):
         tr.close()
     repair.strip(ui, repo, [node])
+
+def initialdescription(ctx):
+    """initial description used in editor template
+
+    3rd party code could change this to edit the text. For example, remove
+    "Differential Revision" line in Phabricator use-case.
+    """
+    return ctx.description()
+
+def nextdescription(prevdesc, ctx):
+    """calculate description used in next editor prompt
+
+    prevdesc: previous description used in editor
+    ctx: changeset just committed
+    """
+    # Remove paragraphs already committed as new messages.
+    # NOTE: bdiff.c hardcodes line-level diff, which should probably be
+    # changed. For now we workaround that by some "encoding" so a "line" bdiff
+    # sees is a "paragraph" originally.
+    encode = lambda s: s.replace('\n', '\0').replace('\0\0', '\n')
+    decode = lambda s: s.replace('\n', '\n\n').replace('\0', '\n')
+    a = encode(prevdesc + '\n\n')
+    b = encode(ctx.description() + '\n\n')
+    alines = mdiff.splitnewlines(a)
+    paragraphs = []
+    for (a1, a2, b1, b2), stype in mdiff.allblocks(a, b, lines1=alines):
+        if stype == '!':
+            paragraphs.append(decode(''.join(alines[a1:a2])))
+    return ''.join(paragraphs).rstrip()
+
+def indent(text):
+    """indent text by 2 spaces"""
+    return ''.join('  %s' % l for l in text.splitlines(True))
diff --git a/tests/test-split.t b/tests/test-split.t
--- a/tests/test-split.t
+++ b/tests/test-split.t
@@ -69,4 +69,5 @@ Split a head
   $ cp -R . ../b
   $ cp -R . ../c
+  $ cp -R . ../e
 
   $ hg bookmark r3
@@ -121,4 +122,7 @@ Split a head
   EDITOR: a2
   EDITOR: 
+  EDITOR: HG: This is the commit message for the #1 changeset split from:
+  EDITOR: HG:   1df0d5c5a3ab: a2
+  EDITOR: 
   EDITOR: 
   EDITOR: HG: Enter commit message.  Lines beginning with 'HG:' are removed.
@@ -140,4 +144,9 @@ Split a head
   EDITOR: a2
   EDITOR: 
+  EDITOR: HG: This is the commit message for the #2 changeset split from:
+  EDITOR: HG:   1df0d5c5a3ab: a2
+  EDITOR: HG: Previous split commit messages are:
+  EDITOR: HG:   split 1
+  EDITOR: 
   EDITOR: 
   EDITOR: HG: Enter commit message.  Lines beginning with 'HG:' are removed.
@@ -158,4 +167,11 @@ Split a head
   EDITOR: a2
   EDITOR: 
+  EDITOR: HG: This is the commit message for the #3 changeset split from:
+  EDITOR: HG:   1df0d5c5a3ab: a2
+  EDITOR: HG: Previous split commit messages are:
+  EDITOR: HG:   split 2
+  EDITOR: HG: --
+  EDITOR: HG:   split 1
+  EDITOR: 
   EDITOR: 
   EDITOR: HG: Enter commit message.  Lines beginning with 'HG:' are removed.
@@ -414,2 +430,152 @@ Split a non-head without rebase
   
 #endif
+
+Used paragraph in commit message will be removed from next prompt:
+
+  $ cd $TESTTMP/e
+  $ hg up tip -q
+  $ cat > message <<EOF
+  > topic: some summary
+  > 
+  > This patch contains two features:
+  > 
+  > - Feature A
+  >   Some description
+  > 
+  > - Feature B
+  >   Some description
+  > 
+  > After this patch, bug XYZ will be fixed.
+  > EOF
+  $ hg commit -m "`cat message`" --amend -q
+  $ cat > $TESTTMP/messages <<EOF
+  > topic: some summary
+  > 
+  > This patch contains one single part:
+  > 
+  > - Feature B
+  >   Some description
+  > --
+  > topic: add some feature B
+  > 
+  > This patch adds the missing part:
+  > 
+  > - Feature A
+  >   Some description
+  > 
+  > After this patch, bug XYZ will be fixed.
+  > --
+  > finalize: write something
+  > EOF
+
+  $ cat <<EOF | hg split tip
+  > y
+  > y
+  > y
+  > y
+  > y
+  > y
+  > EOF
+  diff --git a/a b/a
+  1 hunks, 1 lines changed
+  examine changes to 'a'? [Ynesfdaq?] y
+  
+  @@ -5,1 +5,1 @@ 4
+  -5
+  +55
+  record this change to 'a'? [Ynesfdaq?] y
+  
+  EDITOR: topic: some summary
+  EDITOR: 
+  EDITOR: This patch contains two features:
+  EDITOR: 
+  EDITOR: - Feature A
+  EDITOR:   Some description
+  EDITOR: 
+  EDITOR: - Feature B
+  EDITOR:   Some description
+  EDITOR: 
+  EDITOR: After this patch, bug XYZ will be fixed.
+  EDITOR: 
+  EDITOR: HG: This is the commit message for the #1 changeset split from:
+  EDITOR: HG:   c4f49f93bd45: topic: some summary
+  EDITOR: 
+  EDITOR: 
+  EDITOR: HG: Enter commit message.  Lines beginning with 'HG:' are removed.
+  EDITOR: HG: Leave message empty to abort commit.
+  EDITOR: HG: --
+  EDITOR: HG: user: test
+  EDITOR: HG: branch 'default'
+  EDITOR: HG: changed a
+  created new head
+  diff --git a/a b/a
+  1 hunks, 1 lines changed
+  examine changes to 'a'? [Ynesfdaq?] y
+  
+  @@ -3,1 +3,1 @@ 2
+  -3
+  +33
+  record this change to 'a'? [Ynesfdaq?] y
+  
+  EDITOR: This patch contains two features:
+  EDITOR: 
+  EDITOR: - Feature A
+  EDITOR:   Some description
+  EDITOR: 
+  EDITOR: After this patch, bug XYZ will be fixed.
+  EDITOR: 
+  EDITOR: HG: This is the commit message for the #2 changeset split from:
+  EDITOR: HG:   c4f49f93bd45: topic: some summary
+  EDITOR: HG: Previous split commit messages are:
+  EDITOR: HG:   topic: some summary
+  EDITOR: HG:   
+  EDITOR: HG:   This patch contains one single part:
+  EDITOR: HG:   
+  EDITOR: HG:   - Feature B
+  EDITOR: HG:     Some description
+  EDITOR: 
+  EDITOR: 
+  EDITOR: HG: Enter commit message.  Lines beginning with 'HG:' are removed.
+  EDITOR: HG: Leave message empty to abort commit.
+  EDITOR: HG: --
+  EDITOR: HG: user: test
+  EDITOR: HG: branch 'default'
+  EDITOR: HG: changed a
+  diff --git a/a b/a
+  1 hunks, 1 lines changed
+  examine changes to 'a'? [Ynesfdaq?] y
+  
+  @@ -1,1 +1,1 @@
+  -1
+  +11
+  record this change to 'a'? [Ynesfdaq?] y
+  
+  EDITOR: This patch contains two features:
+  EDITOR: 
+  EDITOR: HG: This is the commit message for the #3 changeset split from:
+  EDITOR: HG:   c4f49f93bd45: topic: some summary
+  EDITOR: HG: Previous split commit messages are:
+  EDITOR: HG:   topic: add some feature B
+  EDITOR: HG:   
+  EDITOR: HG:   This patch adds the missing part:
+  EDITOR: HG:   
+  EDITOR: HG:   - Feature A
+  EDITOR: HG:     Some description
+  EDITOR: HG:   
+  EDITOR: HG:   After this patch, bug XYZ will be fixed.
+  EDITOR: HG: --
+  EDITOR: HG:   topic: some summary
+  EDITOR: HG:   
+  EDITOR: HG:   This patch contains one single part:
+  EDITOR: HG:   
+  EDITOR: HG:   - Feature B
+  EDITOR: HG:     Some description
+  EDITOR: 
+  EDITOR: 
+  EDITOR: HG: Enter commit message.  Lines beginning with 'HG:' are removed.
+  EDITOR: HG: Leave message empty to abort commit.
+  EDITOR: HG: --
+  EDITOR: HG: user: test
+  EDITOR: HG: branch 'default'
+  EDITOR: HG: changed a
+  saved backup bundle to $TESTTMP/e/.hg/strip-backup/c4f49f93bd45-b340b438-backup.hg (glob) (?)


More information about the Mercurial-devel mailing list