Subject: [StGit PATCH 2/2] Implement "stg refresh --edit" again

From: Karl Hasselström <[email-masked]>
Date: Fri, 04 Jul 2008 08:40:36 +0200

The -e/--edit flag to "stg refresh" was dropped between v0.13 and
v0.14, causing severe user dissatisfaction. This patch restores it,
along with -m/--message, -f/--file, --sign, --ack, --author,
--authname, --authemail, and --authdate.

I omitted the --committer options on purpose; I think they are a
mistake. Falsifying the committer info is not a common operation, and
if one wishes to do it for some reason, one can always set the
GIT_COMMITTER_* environment variables.

Signed-off-by: Karl Hasselström <[email-masked]>

---

 stgit/commands/coalesce.py |    2 +-
 stgit/commands/edit.py     |    3 ++-
 stgit/commands/new.py      |    3 ++-
 stgit/commands/refresh.py  |   44 +++++++++++++++++++++++++++++++++-----------
 stgit/utils.py             |   18 ++++++++++++------
 5 files changed, 50 insertions(+), 20 deletions(-)

diff --git a/stgit/commands/coalesce.py b/stgit/commands/coalesce.py
index 1a34934..f3954ce 100644
--- a/stgit/commands/coalesce.py
+++ b/stgit/commands/coalesce.py
@@ -35,7 +35,7 @@ done a sequence of pushes and pops yourself."""
 
 directory = common.DirectoryHasRepositoryLib()
 options = [make_option('-n', '--name', help = 'name of coalesced patch')
-           ] + utils.make_message_options()
+           ] + utils.make_message_options(save_template = True)
 
 class SaveTemplateDone(Exception):
     pass
diff --git a/stgit/commands/edit.py b/stgit/commands/edit.py
index a9e8991..a83ec9c 100644
--- a/stgit/commands/edit.py
+++ b/stgit/commands/edit.py
@@ -60,7 +60,8 @@ options = [make_option('-d', '--diff',
                        action = 'store_true'),
            make_option('-e', '--edit', action = 'store_true',
                        help = 'invoke interactive editor'),
-           ] + (utils.make_sign_options() + utils.make_message_options()
+           ] + (utils.make_sign_options()
+                + utils.make_message_options(save_template = True)
                 + utils.make_author_committer_options()
                 + utils.make_diff_opts_option())
 
diff --git a/stgit/commands/new.py b/stgit/commands/new.py
index c4ee4e1..f468ab0 100644
--- a/stgit/commands/new.py
+++ b/stgit/commands/new.py
@@ -38,7 +38,8 @@ line of the commit message."""
 
 directory = common.DirectoryHasRepositoryLib()
 options = (utils.make_author_committer_options()
-           + utils.make_message_options() + utils.make_sign_options())
+           + utils.make_message_options(save_template = True)
+           + utils.make_sign_options())
 
 def func(parser, options, args):
     """Create a new patch."""
diff --git a/stgit/commands/refresh.py b/stgit/commands/refresh.py
index 7afc55e..37cb559 100644
--- a/stgit/commands/refresh.py
+++ b/stgit/commands/refresh.py
@@ -20,7 +20,7 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 
 from optparse import make_option
 from stgit.commands import common
-from stgit.lib import git, transaction
+from stgit.lib import git, transaction, edit
 from stgit import utils
 
 help = 'generate a new commit for the current patch'
@@ -53,7 +53,11 @@ options = [make_option('-u', '--update', action = 'store_true',
                        help = ('only include changes from the index, not'
                                ' from the work tree')),
            make_option('-p', '--patch',
-                       help = 'refresh PATCH instead of the topmost patch')]
+                       help = 'refresh PATCH instead of the topmost patch'),
+           make_option('-e', '--edit', action = 'store_true',
+                       help = 'invoke an editor for the patch description')
+           ] + (utils.make_message_options(save_template = False)
+                + utils.make_sign_options() + utils.make_author_options())
 
 def get_patch(stack, given_patch):
     """Get the name of the patch we are to refresh."""
@@ -116,9 +120,12 @@ def make_temp_patch(stack, patch_name, paths, temp_index):
     return trans.run(stack.repository.default_iw,
                      print_current_patch = False), temp_name
 
-def absorb_applied(trans, iw, patch_name, temp_name):
+def absorb_applied(trans, iw, patch_name, temp_name, edit_fun):
     """Absorb the temp patch (C{temp_name}) into the given patch
-    (C{patch_name}), which must be applied.
+    (C{patch_name}), which must be applied. If the absorption
+    succeeds, call C{edit_fun} on the resulting
+    L{CommitData<stgit.lib.git.CommitData>} before committing it and
+    commit the return value.
 
     @return: C{True} if we managed to absorb the temp patch, C{False}
              if we had to leave it for the user to deal with."""
@@ -136,7 +143,7 @@ def absorb_applied(trans, iw, patch_name, temp_name):
         temp_cd = trans.patches[temp_name].data
         assert trans.patches[patch_name] == temp_cd.parent
         trans.patches[patch_name] = trans.stack.repository.commit(
-            trans.patches[patch_name].data.set_tree(temp_cd.tree))
+            edit_fun(trans.patches[patch_name].data.set_tree(temp_cd.tree)))
         popped = trans.delete_patches(lambda pn: pn == temp_name, quiet = True)
         assert not popped # the temp patch was topmost
         temp_absorbed = True
@@ -148,9 +155,12 @@ def absorb_applied(trans, iw, patch_name, temp_name):
         pass
     return temp_absorbed
 
-def absorb_unapplied(trans, iw, patch_name, temp_name):
+def absorb_unapplied(trans, iw, patch_name, temp_name, edit_fun):
     """Absorb the temp patch (C{temp_name}) into the given patch
-    (C{patch_name}), which must be unapplied.
+    (C{patch_name}), which must be unapplied. If the absorption
+    succeeds, call C{edit_fun} on the resulting
+    L{CommitData<stgit.lib.git.CommitData>} before committing it and
+    commit the return value.
 
     @param iw: Not used.
     @return: C{True} if we managed to absorb the temp patch, C{False}
@@ -174,7 +184,7 @@ def absorb_unapplied(trans, iw, patch_name, temp_name):
         # It worked. Refresh the patch with the new tree, and delete
         # the temp patch.
         trans.patches[patch_name] = trans.stack.repository.commit(
-            patch_cd.set_tree(new_tree))
+            edit_fun(patch_cd.set_tree(new_tree)))
         popped = trans.delete_patches(lambda pn: pn == temp_name, quiet = True)
         assert not popped # the temp patch was not applied
         return True
@@ -183,13 +193,13 @@ def absorb_unapplied(trans, iw, patch_name, temp_name):
         # leave the temp patch for the user.
         return False
 
-def absorb(stack, patch_name, temp_name):
+def absorb(stack, patch_name, temp_name, edit_fun):
     """Absorb the temp patch into the target patch."""
     trans = transaction.StackTransaction(stack, 'refresh')
     iw = stack.repository.default_iw
     f = { True: absorb_applied, False: absorb_unapplied
           }[patch_name in trans.applied]
-    if f(trans, iw, patch_name, temp_name):
+    if f(trans, iw, patch_name, temp_name, edit_fun):
         def info_msg(): pass
     else:
         def info_msg():
@@ -223,4 +233,16 @@ def func(parser, options, args):
         stack, patch_name, paths, temp_index = path_limiting)
     if retval != utils.STGIT_SUCCESS:
         return retval
-    return absorb(stack, patch_name, temp_name)
+    def edit_fun(cd):
+        cd, failed_diff = edit.auto_edit_patch(
+            stack.repository, cd, msg = options.message, contains_diff = False,
+            author = options.author, committer = lambda p: p,
+            sign_str = options.sign_str)
+        assert not failed_diff
+        if options.edit:
+            cd, failed_diff = edit.interactive_edit_patch(
+                stack.repository, cd, edit_diff = False,
+                diff_flags = [], replacement_diff = None)
+            assert not failed_diff
+        return cd
+    return absorb(stack, patch_name, temp_name, edit_fun)
diff --git a/stgit/utils.py b/stgit/utils.py
index 2983ea8..c40e666 100644
--- a/stgit/utils.py
+++ b/stgit/utils.py
@@ -264,13 +264,13 @@ def add_sign_line(desc, sign_str, name, email):
         desc = desc + '\n'
     return '%s\n%s\n' % (desc, sign_str)
 
-def make_message_options():
+def make_message_options(save_template):
     def no_dup(parser):
         if parser.values.message != None:
             raise optparse.OptionValueError(
                 'Cannot give more than one --message or --file')
     def no_combine(parser):
-        if (parser.values.message != None
+        if (save_template and parser.values.message != None
             and parser.values.save_template != None):
             raise optparse.OptionValueError(
                 'Cannot give both --message/--file and --save-template')
@@ -299,15 +299,18 @@ def make_message_options():
         parser.values.save_template = w
         no_combine(parser)
     m = optparse.make_option
-    return [m('-m', '--message', action = 'callback', callback = msg_callback,
+    opts = [m('-m', '--message', action = 'callback', callback = msg_callback,
               dest = 'message', type = 'string',
               help = 'use MESSAGE instead of invoking the editor'),
             m('-f', '--file', action = 'callback', callback = file_callback,
               dest = 'message', type = 'string', metavar = 'FILE',
-              help = 'use FILE instead of invoking the editor'),
+              help = 'use FILE instead of invoking the editor')]
+    if save_template:
+        opts.append(
             m('--save-template', action = 'callback', callback = templ_callback,
               metavar = 'FILE', dest = 'save_template', type = 'string',
-              help = 'save the message template to FILE and exit')]
+              help = 'save the message template to FILE and exit'))
+    return opts
 
 def make_diff_opts_option():
     def diff_opts_callback(option, opt_str, value, parser):
@@ -368,8 +371,11 @@ def make_person_options(person, short):
                 callback_args = (f,), help = 'set the %s %s' % (person, f))
                for f in ['name', 'email', 'date']])
 
+def make_author_options():
+    return make_person_options('author', 'auth')
+
 def make_author_committer_options():
-    return (make_person_options('author', 'auth')
+    return (make_author_options()
             + make_person_options('committer', 'comm'))
 
 # Exit codes.