Patchwork [2,of,2] split: improve commit message prompt

login
register
mail settings
Submitter Jun Wu
Date June 25, 2017, 5:45 a.m.
Message ID <f3821b7ac6f9c4a47abf.1498369502@x1c>
Download mbox | patch
Permalink /patch/21694/
State Superseded
Headers show

Comments

Jun Wu - June 25, 2017, 5:45 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1498368118 25200
#      Sat Jun 24 22:21:58 2017 -0700
# Node ID f3821b7ac6f9c4a47abf356e1a51719ce02a007f
# Parent  c5c2d312b8293e9488344b39c0889faa3c4442eb
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r f3821b7ac6f9
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.

Patch

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,
@@ -130,15 +131,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:
@@ -167,2 +178,35 @@  def dorebase(ui, repo, tr, src, dest):
     rebase.rebase(ui, repo, rev=[revsetlang.formatspec('%ld', src)],
                   dest=revsetlang.formatspec('%d', dest))
+
+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.
@@ -436,2 +452,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
+