Refactor stgit.commands.edit
authorKarl Hasselström <kha@treskal.com>
Sun, 21 Sep 2008 12:17:43 +0000 (14:17 +0200)
committerKarl Hasselström <kha@treskal.com>
Sun, 21 Sep 2008 12:19:07 +0000 (14:19 +0200)
Reorganize a few existing functions, and break out stuff from the main
function into subroutines.

While we're at it, move one of the old and all of the new functions to
stgit.lib.edit, so that we can use them in a later patch to implement
"stg refresh --edit".

This fixes one of the known failures in t3300-edit.

Signed-off-by: Karl Hasselström <kha@treskal.com>
stgit/commands/common.py
stgit/commands/diff.py
stgit/commands/edit.py
stgit/commands/export.py
stgit/commands/files.py
stgit/commands/imprt.py
stgit/commands/mail.py
stgit/git.py
stgit/lib/edit.py [new file with mode: 0644]
stgit/lib/git.py
t/t3300-edit.sh

index 2bf6ca5..15fdde2 100644 (file)
@@ -419,12 +419,15 @@ def parse_mail(msg):
 
     return (descr, authname, authemail, authdate, diff)
 
-def parse_patch(text):
+def parse_patch(text, contains_diff):
     """Parse the input text and return (description, authname,
     authemail, authdate, diff)
     """
-    descr, diff = __split_descr_diff(text)
-    descr, authname, authemail, authdate = __parse_description(descr)
+    if contains_diff:
+        (text, diff) = __split_descr_diff(text)
+    else:
+        diff = None
+    (descr, authname, authemail, authdate) = __parse_description(text)
 
     # we don't yet have an agreed place for the creation date.
     # Just return None
index 38de3a1..05f4f4c 100644 (file)
@@ -23,6 +23,7 @@ from stgit.commands.common import *
 from stgit.utils import *
 from stgit.out import *
 from stgit import argparse, stack, git
+from stgit.lib import git as gitlib
 
 help = 'Show the tree diff'
 kind = 'wc'
@@ -71,7 +72,7 @@ def func(parser, options, args):
                         rev2 and git_id(crt_series, rev2),
                         diff_flags = options.diff_flags)
     if options.stat:
-        out.stdout_raw(git.diffstat(diff_str) + '\n')
+        out.stdout_raw(gitlib.diffstat(diff_str) + '\n')
     else:
         if diff_str:
             pager(diff_str)
index b6ef6c7..9043c0c 100644 (file)
@@ -21,7 +21,7 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 from stgit.argparse import opt
 from stgit import argparse, git, utils
 from stgit.commands import common
-from stgit.lib import git as gitlib, transaction
+from stgit.lib import git as gitlib, transaction, edit
 from stgit.out import *
 
 help = 'edit a patch description or diff'
@@ -64,45 +64,6 @@ options = [
 
 directory = common.DirectoryHasRepositoryLib()
 
-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:
-        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, quiet = False)
-        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.
     """
@@ -122,44 +83,26 @@ def func(parser, options, args):
 
     cd = orig_cd = stack.patches.get(patchname).commit.data
 
-    # 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.
-    a, c = options.author(cd.author), options.committer(cd.committer)
-    if (a, c) != (cd.author, cd.committer):
-        cd = cd.set_author(a).set_committer(c)
-
-    # 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))
+    cd, failed_diff = edit.auto_edit_patch(
+        stack.repository, cd, msg = options.message, contains_diff = True,
+        author = options.author, committer = options.committer,
+        sign_str = options.sign_str)
 
     if options.save_template:
         options.save_template(
-            patch_desc(stack.repository, cd, failed_diff,
-                       options.diff, options.diff_flags))
+            edit.patch_desc(stack.repository, cd,
+                            options.diff, options.diff_flags, failed_diff))
         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))
+        cd, failed_diff = edit.interactive_edit_patch(
+            stack.repository, cd, options.diff, options.diff_flags, failed_diff)
 
     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.write(edit.patch_desc(stack.repository, cd,
+                                options.diff, options.diff_flags, failed_diff))
         f.close()
         out.error('Edited patch did not apply.',
                   'It has been saved to "%s".' % fn)
index 8d05996..c7ed802 100644 (file)
@@ -143,7 +143,7 @@ def func(parser, options, args):
         tmpl_dict = {'description': descr,
                      'shortdescr': short_descr,
                      'longdescr': long_descr,
-                     'diffstat': git.diffstat(diff).rstrip(),
+                     'diffstat': gitlib.diffstat(diff).rstrip(),
                      'authname': cd.author.name,
                      'authemail': cd.author.email,
                      'authdate': cd.author.date.isoformat(),
index a7576e9..d63a33e 100644 (file)
@@ -22,6 +22,7 @@ from stgit.commands.common import *
 from stgit.utils import *
 from stgit.out import *
 from stgit import argparse, stack, git
+from stgit.lib import git as gitlib
 
 help = 'Show the files modified by a patch (or the current patch)'
 kind = 'patch'
@@ -56,7 +57,7 @@ def func(parser, options, args):
     rev2 = git_id(crt_series, '%s' % patch)
 
     if options.stat:
-        out.stdout_raw(git.diffstat(git.diff(rev1 = rev1, rev2 = rev2)) + '\n')
+        out.stdout_raw(gitlib.diffstat(git.diff(rev1 = rev1, rev2 = rev2)) + '\n')
     elif options.bare:
         out.stdout_raw(git.barefiles(rev1, rev2) + '\n')
     else:
index 103ebb0..9f2df05 100644 (file)
@@ -212,7 +212,7 @@ def __import_file(filename, options, patch = None):
                  parse_mail(msg)
     else:
         message, author_name, author_email, author_date, diff = \
-                 parse_patch(f.read())
+                 parse_patch(f.read(), contains_diff = True)
 
     if filename:
         f.close()
index 6948a1e..e0a5521 100644 (file)
@@ -24,6 +24,7 @@ from stgit.out import *
 from stgit import argparse, stack, git, version, templates
 from stgit.config import config
 from stgit.run import Run
+from stgit.lib import git as gitlib
 
 help = 'Send a patch or series of patches by e-mail'
 kind = 'patch'
@@ -369,7 +370,7 @@ def __build_cover(tmpl, patches, msg_id, options):
                  'number':       number_str,
                  'shortlog':     stack.shortlog(crt_series.get_patch(p)
                                                 for p in patches),
-                 'diffstat':     git.diffstat(git.diff(
+                 'diffstat':     gitlib.diffstat(git.diff(
                      rev1 = git_id(crt_series, '%s^' % patches[0]),
                      rev2 = git_id(crt_series, '%s' % patches[-1])))}
 
@@ -459,7 +460,7 @@ def __build_message(tmpl, patch, patch_nr, total_nr, msg_id, ref_id, options):
                  # for backward template compatibility
                  'endofheaders': '',
                  'diff':         diff,
-                 'diffstat':     git.diffstat(diff),
+                 'diffstat':     gitlib.diffstat(diff),
                  # for backward template compatibility
                  'date':         '',
                  'version':      version_str,
index ee31ecd..4d01fc2 100644 (file)
@@ -669,10 +669,6 @@ def diff(files = None, rev1 = 'HEAD', rev2 = None, diff_flags = [],
     else:
         return ''
 
-def diffstat(diff):
-    """Return the diffstat of the supplied diff."""
-    return GRun('apply', '--stat', '--summary').raw_input(diff).raw_output()
-
 def files(rev1, rev2, diff_flags = []):
     """Return the files modified between rev1 and rev2
     """
diff --git a/stgit/lib/edit.py b/stgit/lib/edit.py
new file mode 100644 (file)
index 0000000..c8d29f6
--- /dev/null
@@ -0,0 +1,99 @@
+"""This module contains utility functions for patch editing."""
+
+from stgit import utils
+from stgit.commands import common
+from stgit.lib import git
+
+def update_patch_description(repo, cd, text, contains_diff):
+    """Update the given L{CommitData<stgit.lib.git.CommitData>} with the
+    given text description, which may contain author name and time
+    stamp in addition to a new commit message. If C{contains_diff} is
+    true, it may also contain a replacement diff.
+
+    Return a pair: the new L{CommitData<stgit.lib.git.CommitData>};
+    and the diff text if it didn't apply, or C{None} otherwise."""
+    (message, authname, authemail, authdate, diff
+     ) = common.parse_patch(text, contains_diff)
+    a = cd.author
+    for val, setter in [(authname, 'set_name'), (authemail, 'set_email'),
+                        (git.Date.maybe(authdate), 'set_date')]:
+        if val != None:
+            a = getattr(a, setter)(val)
+    cd = cd.set_message(message).set_author(a)
+    failed_diff = None
+    if diff:
+        tree = repo.apply(cd.parent.data.tree, diff, quiet = False)
+        if tree == None:
+            failed_diff = diff
+        else:
+            cd = cd.set_tree(tree)
+    return cd, failed_diff
+
+def patch_desc(repo, cd, append_diff, diff_flags, replacement_diff):
+    """Return a description text for the patch, suitable for editing
+    and/or reimporting with L{update_patch_description()}.
+
+    @param cd: The L{CommitData<stgit.lib.git.CommitData>} to generate
+               a description of
+    @param append_diff: Whether to append the patch diff to the
+                        description
+    @type append_diff: C{bool}
+    @param diff_flags: Extra parameters to pass to C{git diff}
+    @param replacement_diff: Diff text to use; or C{None} if it should
+                             be computed from C{cd}
+    @type replacement_diff: C{str} or C{None}"""
+    desc = ['From: %s <%s>' % (cd.author.name, cd.author.email),
+            'Date: %s' % cd.author.date.isoformat(),
+            '',
+            cd.message]
+    if append_diff:
+        if replacement_diff:
+            diff = replacement_diff
+        else:
+            just_diff = repo.diff_tree(cd.parent.data.tree, cd.tree, diff_flags)
+            diff = '\n'.join([git.diffstat(just_diff), just_diff])
+        desc += ['---', '', diff]
+    return '\n'.join(desc)
+
+def interactive_edit_patch(repo, cd, edit_diff, diff_flags, replacement_diff):
+    """Edit the patch interactively. If C{edit_diff} is true, edit the
+    diff as well. If C{replacement_diff} is not C{None}, it contains a
+    diff to edit instead of the patch's real diff.
+
+    Return a pair: the new L{CommitData<stgit.lib.git.CommitData>};
+    and the diff text if it didn't apply, or C{None} otherwise."""
+    return update_patch_description(
+        repo, cd, utils.edit_string(
+            patch_desc(repo, cd, edit_diff, diff_flags, replacement_diff),
+            '.stgit-edit.' + ['txt', 'patch'][bool(edit_diff)]),
+        edit_diff)
+
+def auto_edit_patch(repo, cd, msg, contains_diff, author, committer, sign_str):
+    """Edit the patch noninteractively in a couple of ways:
+
+         - If C{msg} is not C{None}, parse it to find a replacement
+           message, and possibly also replacement author and
+           timestamp. If C{contains_diff} is true, also look for a
+           replacement diff.
+
+         - C{author} and C{committer} are two functions that take the
+           original L{Person<stgit.lib.git.Person>} value as argument,
+           and return the new one.
+
+         - C{sign_str}, if not C{None}, is a sign string to append to
+           the message.
+
+    Return a pair: the new L{CommitData<stgit.lib.git.CommitData>};
+    and the diff text if it didn't apply, or C{None} otherwise."""
+    if msg == None:
+        failed_diff = None
+    else:
+        cd, failed_diff = update_patch_description(repo, cd, msg, contains_diff)
+    a, c = author(cd.author), committer(cd.committer)
+    if (a, c) != (cd.author, cd.committer):
+        cd = cd.set_author(a).set_committer(c)
+    if sign_str != None:
+        cd = cd.set_message(utils.add_sign_line(
+                cd.message, sign_str, git.Person.committer().name,
+                git.Person.committer().email))
+    return cd, failed_diff
index 22fdce4..0a208ef 100644 (file)
@@ -900,3 +900,8 @@ class Branch(object):
         repository.run(['git', 'branch', create_at.sha1]).discard_output()
 
         return cls(repository, name)
+
+def diffstat(diff):
+    """Return the diffstat of the supplied diff."""
+    return run.Run('git', 'apply', '--stat', '--summary'
+                   ).raw_input(diff).raw_output()
index 5772e48..5c2d32e 100755 (executable)
@@ -93,7 +93,7 @@ mkeditor ()
 {
     cat > "$1" <<EOF
 #!/bin/sh
-printf "\n$1\n" >> "\$1"
+printf "\n$1" >> "\$1"
 EOF
     chmod a+x "$1"
 }
@@ -148,7 +148,7 @@ git config --unset core.editor
 git config --unset stgit.editor
 
 mkeditor twoliner
-test_expect_failure 'Both noninterative and interactive editing' '
+test_expect_success 'Both noninterative and interactive editing' '
     EDITOR=./twoliner stg edit -e -m "oneliner" p2 &&
     test "$(msg HEAD)" = "oneliner/twoliner"
 '