git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[StGit PATCH 2/3] Convert "stg edit" to the new infrastructure

From
Karl Hasselström <kha@treskal.com>
Date
Feb 1, 2008, 07:50 UTC
Message-ID
<20080201075017.7905.41241.stgit@yoghurt>
In-Reply-To
<20080201074708.7905.98305.stgit@yoghurt>

The --annotate and --undo switches were dropped in the conversion. --annotate could be re-added, but --undo is more problematic since the command will now rewrite any applied patches on top of the edited patch. It seems best to leave this job to the fabled general undo command, expected Real Soon Now.

In addition to the usual improvements from the new infrastructure, this patch has some additional benefits:

  * There's a new -e/--edit flag, which forces interactive editing
    even if options such as --sign or --author are given. (Normally,
    interactive editing is skipped if the patch is modified with a
    commandline option.)
  * It's now possible to edit any patch, including unapplied patches.
    Even diff editing works for all patches, including unapplied
    patches. (In fact, editing unapplied patches is slightly safer,
    since they don't mind a dirty index/worktree.)
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
 stgit/commands/edit.py |  309 ++++++++++++++++++++++--------------------------
 1 files changed, 142 insertions(+), 167 deletions(-)
diff --git a/stgit/commands/edit.py b/stgit/commands/edit.py
index 9915e49..7daf156 100644
--- a/stgit/commands/edit.py
+++ b/stgit/commands/edit.py
@@ -18,14 +18,12 @@ along with this program; if not, write to the Free Software
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 """
 
-from optparse import OptionParser, make_option
-from email.Utils import formatdate
+from optparse import make_option
 
-from stgit.commands.common import *
-from stgit.utils import *
+from stgit import git, utils
+from stgit.commands import common
+from stgit.lib import git as gitlib, transaction
 from stgit.out import *
-from stgit import stack, git
-
 
 help = 'edit a patch description or diff'
 usage = """%prog [options] [<patch>]
@@ -49,23 +47,19 @@ separator:
   Diff text
 
 Command-line options can be used to modify specific information
-without invoking the editor.
+without invoking the editor. (With the --edit option, the editor is
+invoked even if such command-line options are given.)
 
-If the patch diff is edited but the patch application fails, the
-rejected patch is stored in the .stgit-failed.patch file (and also in
-.stgit-edit.{diff,txt}). The edited patch can be replaced with one of
-these files using the '--file' and '--diff' options.
-"""
+If the patch diff is edited but does not apply, no changes are made to
+the patch at all. The edited patch is saved to a file which you can
+feed to "stg edit --file", once you have made sure it does apply."""
 
-directory = DirectoryGotoToplevel()
+directory = common.DirectoryHasRepositoryLib()
 options = [make_option('-d', '--diff',
                        help = 'edit the patch diff',
                        action = 'store_true'),
-           make_option('--undo',
-                       help = 'revert the commit generated by the last edit',
-                       action = 'store_true'),
-           make_option('-a', '--annotate', metavar = 'NOTE',
-                       help = 'annotate the patch log entry'),
+           make_option('-e', '--edit', action = 'store_true',
+                       help = 'invoke interactive editor'),
            make_option('--author', metavar = '"NAME <EMAIL>"',
                        help = 'replae the author details with "NAME <EMAIL>"'),
            make_option('--authname',
@@ -78,162 +72,143 @@ options = [make_option('-d', '--diff',
                        help = 'replace the committer name with COMMNAME'),
            make_option('--commemail',
                        help = 'replace the committer e-mail with COMMEMAIL')
-           ] + (make_sign_options() + make_message_options()
-                + make_diff_opts_option())
-
-def __update_patch(pname, text, options):
-    """Update the current patch from the given text.
-    """
-    patch = crt_series.get_patch(pname)
-
-    bottom = patch.get_bottom()
-    top = patch.get_top()
-
-    if text:
-        (message, author_name, author_email, author_date, diff
-         ) = parse_patch(text)
-    else:
-        message = author_name = author_email = author_date = diff = None
-
-    out.start('Updating patch "%s"' % pname)
-
-    if options.diff:
-        git.switch(bottom)
-        try:
-            git.apply_patch(diff = diff)
-        except:
-            # avoid inconsistent repository state
-            git.switch(top)
-            raise
-
-    def c(a, b):
-        if a != None:
-            return a
-        return b
-    crt_series.refresh_patch(message = message,
-                             author_name = c(options.authname, author_name),
-                             author_email = c(options.authemail, author_email),
-                             author_date = c(options.authdate, author_date),
-                             committer_name = options.commname,
-                             committer_email = options.commemail,
-                             backup = True, sign_str = options.sign_str,
-                             log = 'edit', notes = options.annotate)
-
-    if crt_series.empty_patch(pname):
-        out.done('empty patch')
-    else:
-        out.done()
-
-def __generate_file(pname, write_fn, options):
-    """Generate a file containing the description to edit
-    """
-    patch = crt_series.get_patch(pname)
-
-    # generate the file to be edited
-    descr = patch.get_description().strip()
-    authdate = patch.get_authdate()
-
-    tmpl = 'From: %(authname)s <%(authemail)s>\n'
-    if authdate:
-        tmpl += 'Date: %(authdate)s\n'
-    tmpl += '\n%(descr)s\n'
-
-    tmpl_dict = {
-        'descr': descr,
-        'authname': patch.get_authname(),
-        'authemail': patch.get_authemail(),
-        'authdate': patch.get_authdate()
-        }
-
-    if options.diff:
-        # add the patch diff to the edited file
-        bottom = patch.get_bottom()
-        top = patch.get_top()
+           ] + (utils.make_sign_options() + utils.make_message_options()
+                + utils.make_diff_opts_option())
 
-        tmpl += '---\n\n' \
-                '%(diffstat)s\n' \
-                '%(diff)s'
-
-        tmpl_dict['diff'] = git.diff(rev1 = bottom, rev2 = top,
-                                     diff_flags = options.diff_flags)
-        tmpl_dict['diffstat'] = git.diffstat(tmpl_dict['diff'])
-
-    for key in tmpl_dict:
-        # make empty strings if key is not available
-        if tmpl_dict[key] is None:
-            tmpl_dict[key] = ''
-
-    text = tmpl % tmpl_dict
-
-    # write the file to be edited
-    write_fn(text)
-
-def __edit_update_patch(pname, options):
-    """Edit the given patch interactively.
-    """
-    if options.diff:
-        fname = '.stgit-edit.diff'
+def patch_diff(repository, cd, diff, diff_flags):
+    if diff:
+        diff = repository.diff_tree(cd.parent.data.tree, cd.tree, diff_flags)
+        return '\n'.join([git.diffstat(diff), diff])
     else:
-        fname = '.stgit-edit.txt'
-    def write_fn(text):
-        f = file(fname, 'w')
-        f.write(text)
-        f.close()
-
-    __generate_file(pname, write_fn, options)
-
-    # invoke the editor
-    call_editor(fname)
-
-    __update_patch(pname, file(fname).read(), options)
+        return None
+
+def patch_description(cd, diff):
+    """Generate a string containing the description to edit."""
+
+    desc = ['From: %s <%s>' % (cd.author.name, cd.author.email),
+            'Date: %s' % cd.author.date.isoformat(),
+            '',
+            cd.message]
+    if diff:
+        desc += ['---',
+                 '',
+                diff]
+    return '\n'.join(desc)
+
+def patch_desc(repository, cd, failed_diff, diff, diff_flags):
+    return patch_description(cd, failed_diff or patch_diff(
+            repository, cd, diff, diff_flags))
+
+def update_patch_description(repository, cd, text):
+    message, authname, authemail, authdate, diff = common.parse_patch(text)
+    cd = (cd.set_message(message)
+            .set_author(cd.author.set_name(authname)
+                                 .set_email(authemail)
+                                 .set_date(gitlib.Date.maybe(authdate))))
+    failed_diff = None
+    if diff:
+        tree = repository.apply(cd.parent.data.tree, diff)
+        if tree == None:
+            failed_diff = diff
+        else:
+            cd = cd.set_tree(tree)
+    return cd, failed_diff
 
 def func(parser, options, args):
     """Edit the given patch or the current one.
     """
-    crt_pname = crt_series.get_current()
+    stack = directory.repository.current_stack
 
-    if not args:
-        pname = crt_pname
-        if not pname:
-            raise CmdException, 'No patches applied'
+    if len(args) == 0:
+        if not stack.patchorder.applied:
+            raise common.CmdException(
+                'Cannot edit top patch, because no patches are applied')
+        patchname = stack.patchorder.applied[-1]
     elif len(args) == 1:
-        pname = args[0]
-        if crt_series.patch_unapplied(pname) or crt_series.patch_hidden(pname):
-            raise CmdException, 'Cannot edit unapplied or hidden patches'
-        elif not crt_series.patch_applied(pname):
-            raise CmdException, 'Unknown patch "%s"' % pname
+        [patchname] = args
+        if not stack.patches.exists(patchname):
+            raise common.CmdException('%s: no such patch' % patchname)
     else:
-        parser.error('incorrect number of arguments')
-
-    check_local_changes()
-    check_conflicts()
-    check_head_top_equal(crt_series)
-
-    if pname != crt_pname:
-        # Go to the patch to be edited
-        applied = crt_series.get_applied()
-        between = applied[:applied.index(pname):-1]
-        pop_patches(crt_series, between)
-
-    if options.author:
-        options.authname, options.authemail = name_email(options.author)
-
-    if options.undo:
-        out.start('Undoing the editing of "%s"' % pname)
-        crt_series.undo_refresh()
-        out.done()
-    elif options.save_template:
-        __generate_file(pname, options.save_template, options)
-    elif any([options.message, options.authname, options.authemail,
-              options.authdate, options.commname, options.commemail,
-              options.sign_str]):
-        out.start('Updating patch "%s"' % pname)
-        __update_patch(pname, options.message, options)
-        out.done()
-    else:
-        __edit_update_patch(pname, options)
+        parser.error('Cannot edit more than one patch')
+
+    cd = orig_cd = stack.patches.get(patchname).commit.data
 
-    if pname != crt_pname:
-        # Push the patches back
-        between.reverse()
-        push_patches(crt_series, between)
+    # Read patch from user-provided description.
+    if options.message == None:
+        failed_diff = None
+    else:
+        cd, failed_diff = update_patch_description(stack.repository, cd,
+                                                   options.message)
+
+    # Modify author and committer data.
+    if options.author != None:
+        options.authname, options.authemail = common.name_email(options.author)
+    for p, f, val in [('author', 'name', options.authname),
+                      ('author', 'email', options.authemail),
+                      ('author', 'date', gitlib.Date.maybe(options.authdate)),
+                      ('committer', 'name', options.commname),
+                      ('committer', 'email', options.commemail)]:
+        if val != None:
+            cd = getattr(cd, 'set_' + p)(
+                getattr(getattr(cd, p), 'set_' + f)(val))
+
+    # Add Signed-off-by: or similar.
+    if options.sign_str != None:
+        cd = cd.set_message(utils.add_sign_line(
+                cd.message, options.sign_str, gitlib.Person.committer().name,
+                gitlib.Person.committer().email))
+
+    if options.save_template:
+        options.save_template(
+            patch_desc(stack.repository, cd, failed_diff,
+                       options.diff, options.diff_flags))
+        return utils.STGIT_SUCCESS
+
+    # Let user edit the patch manually.
+    if cd == orig_cd or options.edit:
+        fn = '.stgit-edit.' + ['txt', 'patch'][bool(options.diff)]
+        cd, failed_diff = update_patch_description(
+            stack.repository, cd, utils.edit_string(
+                patch_desc(stack.repository, cd, failed_diff,
+                           options.diff, options.diff_flags),
+                fn))
+
+    def failed():
+        fn = '.stgit-failed.patch'
+        f = file(fn, 'w')
+        f.write(patch_desc(stack.repository, cd, failed_diff,
+                           options.diff, options.diff_flags))
+        f.close()
+        out.error('Edited patch did not apply.',
+                  'It has been saved to "%s".' % fn)
+        return utils.STGIT_COMMAND_ERROR
+
+    # If we couldn't apply the patch, fail without even trying to
+    # effect any of the changes.
+    if failed_diff:
+        return failed()
+
+    # The patch applied, so now we have to rewrite the StGit patch
+    # (and any patches on top of it).
+    iw = stack.repository.default_iw
+    trans = transaction.StackTransaction(stack, 'stg edit')
+    if patchname in trans.applied:
+        popped = trans.applied[trans.applied.index(patchname)+1:]
+        assert not trans.pop_patches(lambda pn: pn in popped)
+    else:
+        popped = []
+    trans.patches[patchname] = stack.repository.commit(cd)
+    try:
+        for pn in popped:
+            trans.push_patch(pn, iw)
+    except transaction.TransactionHalted:
+        pass
+    try:
+        # Either a complete success, or a conflict during push. But in
+        # either case, we've successfully effected the edits the user
+        # asked us for.
+        return trans.run(iw)
+    except transaction.TransactionException:
+        # Transaction aborted -- we couldn't check out files due to
+        # dirty index/worktree. The edits were not carried out.
+        return failed()
Previous: Karl HasselströmNext: Karl Hasselström
Message 24 of 25 in “kha/safe and kha/experimental updated”
  1. Karl HasselströmJan 29, 2008
  2. 0/5 Various cleanupsKarl Hasselström, Jan 29, 2008
  3. 1/5 Create index and worktree objects just onceKarl Hasselström, Jan 29, 2008
  4. 2/5 Wrap excessively long lineKarl Hasselström, Jan 29, 2008
  5. 3/5 Eliminate temp variable that's used just onceKarl Hasselström, Jan 29, 2008
  6. 4/5 Simplify editor selection logicKarl Hasselström, Jan 29, 2008
  7. Peter OberndorferJan 29, 2008
  8. Karl HasselströmJan 30, 2008
  9. Jay SoffianJan 30, 2008
  10. Karl HasselströmJan 30, 2008
  11. 5/5 Let the caller supply the diff text to diffstat()Karl Hasselström, Jan 29, 2008
  12. 0/2 "stg clean" test+bugfixKarl Hasselström, Jan 29, 2008
  13. 1/2 Add test to ensure that "stg clean" preserves conflicting patchesKarl Hasselström, Jan 29, 2008
  14. 2/2 Don't clean away patches with conflictsKarl Hasselström, Jan 29, 2008
  15. 0/4 Rewrite "stg edit" to use new infrastructureKarl Hasselström, Jan 29, 2008
  16. 1/4 Teach new infrastructure about the default author and committerKarl Hasselström, Jan 29, 2008
  17. 2/4 Teach new infrastructure to apply patchesKarl Hasselström, Jan 29, 2008
  18. 3/4 Teach new infrastructure to diff two treesKarl Hasselström, Jan 29, 2008
  19. David KågedalJan 29, 2008
  20. Karl HasselströmJan 29, 2008
  21. 4/4 Convert "stg edit" to the new infrastructureKarl Hasselström, Jan 29, 2008
  22. 0/3 "stg edit" fixupsKarl Hasselström, Feb 1, 2008
  23. 1/3 Parse the date instead of treating it as an opaque stringKarl Hasselström, Feb 1, 2008
  24. 2/3 Convert "stg edit" to the new infrastructureKarl Hasselström, Feb 1, 2008
  25. 3/3 It's possible to edit unapplied patches nowKarl Hasselström, Feb 1, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.