Patchwork [4,of,4,V2] crecord: re-enable reviewing a patch before comitting it

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date March 21, 2016, 1:09 a.m.
Message ID <14d5d22ddda23d54394f.1458522581@Iris>
Download mbox | patch
Permalink /patch/14004/
State Accepted
Headers show

Comments

Jordi Gutiérrez Hermoso - March 21, 2016, 1:09 a.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1458522497 14400
#      Sun Mar 20 21:08:17 2016 -0400
# Node ID 14d5d22ddda23d54394f076088de8e00adb51354
# Parent  bf07fd7c4934b85b2d0e671ba02b6104431e0dda
crecord: re-enable reviewing a patch before comitting it

The "r" option for this feature was copied into Mercurial from
crecord, but the actual implementation never made it into hg until
now. It's a moderately useful feature that allows the user to edit the
patch in a text editor before comitting it for good.

This requires a test, so we must also enable a corresponding testing
'R' option that skips the confirmation dialogue. In addition, we also
need a help text for the editor when reviewing the final patch.

As for why this is a useful feature if we can already edit hunks in an
editor, I would like to offer the following points:

    * editing hunks does not show the entire patch all at once

      ** furthermore, the hunk "tree" in the TUI has no root that could be
         selected for edition

    * it is helpful to be able to see the entire final patch for
      confirmation

      ** within this view, the unselected hunks are hidden, which is
         visusally cleaner

      ** this works as a final review of the complete result, which is
         a bit more difficult to do conceptually via hunk editing

    * this feature was already in crecord, so it was an oversight to
      not bring it to core

    * it works and is consistent with editing hunks

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -207,6 +207,17 @@  def dorecord(ui, repo, commitfunc, cmdsu
             dopatch = fp.tell()
             fp.seek(0)
 
+            # 2.5 optionally review / modify patch in text editor
+            if opts.get('review', False):
+                patchtext = (crecordmod.diffhelptext
+                             + crecordmod.patchhelptext
+                             + fp.read())
+                reviewedpatch = ui.edit(patchtext, "",
+                                        extra={"suffix": ".diff"})
+                fp.truncate(0)
+                fp.write(reviewedpatch)
+                fp.seek(0)
+
             [os.unlink(repo.wjoin(c)) for c in newlyaddedandmodifiedfiles]
             # 3a. apply filtered patch to clean repo  (clean)
             if backups:
diff --git a/mercurial/crecord.py b/mercurial/crecord.py
--- a/mercurial/crecord.py
+++ b/mercurial/crecord.py
@@ -44,6 +44,13 @@  hunkhelptext = _("""#
 # of the hunk are removed, then the edit is aborted and the hunk is left
 # unchanged.
 """)
+
+patchhelptext = _("""#
+# If the patch applies cleanly, the edited patch will immediately
+# be finalised. If it does not apply cleanly, rejects files will be
+# generated. You can use those when you try again.
+""")
+
 try:
     import curses
     import fcntl
@@ -1608,10 +1615,14 @@  are you sure you want to review/edit and
         elif keypressed in ["c"]:
             if self.confirmcommit():
                 return True
+        elif test and keypressed in ['X']:
+            return True
         elif keypressed in ["r"]:
             if self.confirmcommit(review=True):
+                self.opts['review'] = True
                 return True
-        elif test and keypressed in ['X']:
+        elif test and keypressed in ['R']:
+            self.opts['review'] = True
             return True
         elif keypressed in [' '] or (test and keypressed in ["TOGGLE"]):
             self.toggleapply()
diff --git a/tests/test-commit-interactive-curses.t b/tests/test-commit-interactive-curses.t
--- a/tests/test-commit-interactive-curses.t
+++ b/tests/test-commit-interactive-curses.t
@@ -222,4 +222,79 @@  of the edit.
   foo
   hello world
 
+Testing the review option. The entire final filtered patch should show
+up in the editor and be editable. We will unselect the second file and
+the first hunk of the third file. During review, we will decide that
+"lower" sounds better than "bottom", and the final commit should
+reflect this edition.
 
+  $ hg update -C .
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ echo "top" > c
+  $ cat x >> c
+  $ echo "bottom" >> c
+  $ mv c x
+  $ echo "third a" >> a
+  $ echo "we will unselect this" >> b
+
+  $ cat > editor.sh <<EOF
+  > cat "\$1"
+  > cat "\$1" | sed s/bottom/lower/ > tmp
+  > mv tmp "\$1"
+  > EOF
+  $ cat > testModeCommands <<EOF
+  > KEY_DOWN
+  > TOGGLE
+  > KEY_DOWN
+  > f
+  > KEY_DOWN
+  > TOGGLE
+  > R
+  > EOF
+
+  $ HGEDITOR="\"sh\" \"`pwd`/editor.sh\"" hg commit  -i -m "review hunks" -d "0 0"
+  # To remove '-' lines, make them ' ' lines (context).
+  # To remove '+' lines, delete them.
+  # Lines starting with # will be removed from the patch.
+  #
+  # If the patch applies cleanly, the edited patch will immediately
+  # be finalised. If it does not apply cleanly, rejects files will be
+  # generated. You can use those when you try again.
+  diff --git a/a b/a
+  --- a/a
+  +++ b/a
+  @@ -1,2 +1,3 @@
+   a
+   a
+  +third a
+  diff --git a/x b/x
+  --- a/x
+  +++ b/x
+  @@ -1,2 +1,3 @@
+   foo
+   hello world
+  +bottom
+
+  $ hg cat -r . a
+  a
+  a
+  third a
+
+  $ hg cat -r . b
+  x
+  1
+  2
+  3
+  4
+  5
+  6
+  7
+  8
+  9
+  10
+  y
+
+  $ hg cat -r . x
+  foo
+  hello world
+  lower