From ef954fe616e3a8012d247292dc8a82a4e3d8d333 Mon Sep 17 00:00:00 2001 Message-Id: From: Mark Wooding Date: Sun, 21 Sep 2008 14:17:43 +0200 Subject: [PATCH] Refactor stgit.commands.edit MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Organization: Straylight/Edgeware From: Karl Hasselström 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 --- stgit/commands/common.py | 9 ++-- stgit/commands/diff.py | 3 +- stgit/commands/edit.py | 79 +++++--------------------------- stgit/commands/export.py | 2 +- stgit/commands/files.py | 3 +- stgit/commands/imprt.py | 2 +- stgit/commands/mail.py | 5 +- stgit/git.py | 4 -- stgit/lib/edit.py | 99 ++++++++++++++++++++++++++++++++++++++++ stgit/lib/git.py | 5 ++ t/t3300-edit.sh | 4 +- 11 files changed, 132 insertions(+), 83 deletions(-) create mode 100644 stgit/lib/edit.py diff --git a/stgit/commands/common.py b/stgit/commands/common.py index 2bf6ca5..15fdde2 100644 --- a/stgit/commands/common.py +++ b/stgit/commands/common.py @@ -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 diff --git a/stgit/commands/diff.py b/stgit/commands/diff.py index 38de3a1..05f4f4c 100644 --- a/stgit/commands/diff.py +++ b/stgit/commands/diff.py @@ -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) diff --git a/stgit/commands/edit.py b/stgit/commands/edit.py index b6ef6c7..9043c0c 100644 --- a/stgit/commands/edit.py +++ b/stgit/commands/edit.py @@ -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) diff --git a/stgit/commands/export.py b/stgit/commands/export.py index 8d05996..c7ed802 100644 --- a/stgit/commands/export.py +++ b/stgit/commands/export.py @@ -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(), diff --git a/stgit/commands/files.py b/stgit/commands/files.py index a7576e9..d63a33e 100644 --- a/stgit/commands/files.py +++ b/stgit/commands/files.py @@ -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: diff --git a/stgit/commands/imprt.py b/stgit/commands/imprt.py index 103ebb0..9f2df05 100644 --- a/stgit/commands/imprt.py +++ b/stgit/commands/imprt.py @@ -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() diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py index 6948a1e..e0a5521 100644 --- a/stgit/commands/mail.py +++ b/stgit/commands/mail.py @@ -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, diff --git a/stgit/git.py b/stgit/git.py index ee31ecd..4d01fc2 100644 --- a/stgit/git.py +++ b/stgit/git.py @@ -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 index 0000000..c8d29f6 --- /dev/null +++ b/stgit/lib/edit.py @@ -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} 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}; + 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} 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}; + 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} 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}; + 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 diff --git a/stgit/lib/git.py b/stgit/lib/git.py index 22fdce4..0a208ef 100644 --- a/stgit/lib/git.py +++ b/stgit/lib/git.py @@ -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() diff --git a/t/t3300-edit.sh b/t/t3300-edit.sh index 5772e48..5c2d32e 100755 --- a/t/t3300-edit.sh +++ b/t/t3300-edit.sh @@ -93,7 +93,7 @@ mkeditor () { cat > "$1" <> "\$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" ' -- [mdw]