StGit: kha/safe and kha/experimental updated

StGit: kha/safe and kha/experimental updated

[Karl Hasselström]

Both branches updated. I've promoted some experimental patches to
safe, and done some work on new and delete (will post the patches in
reply to this mail).

                                 -+-

The following changes since commit a639e7bbf3a6396254af76062ee88c22e4373340:
  Karl Hasselström (1):
        Don't clean away patches with conflicts

are available in the git repository at:

  git://repo.or.cz/stgit/kha.git safe

Karl Hasselström (11):
      Don't check out files if we don't have index+workdir
      Make documentation less confusing
      Reduce number of backslashes by using raw strings
      Let "stg show" use the unified --diff-opts handling
      Read default diff options from the user's config
      Teach new infrastructure about the default author and committer
      Teach new infrastructure to apply patches
      Teach new infrastructure to diff two trees
      Parse the date instead of treating it as an opaque string
      Convert "stg edit" to the new infrastructure
      Emacs mode: It's possible to edit unapplied patches now

Peter Oberndorfer (1):
      Add an --index option to "stg refresh"

 contrib/stgit.el          |    4 +-
 examples/gitconfig        |    4 +
 stgit/commands/common.py  |    4 +-
 stgit/commands/edit.py    |  309 +++++++++++++++++++++------------------------
 stgit/commands/mail.py    |    4 +-
 stgit/commands/new.py     |    6 +-
 stgit/commands/refresh.py |   25 +++-
 stgit/commands/show.py    |   13 +--
 stgit/lib/git.py          |  126 ++++++++++++++++++-
 stgit/lib/transaction.py  |   13 +-
 stgit/utils.py            |    3 +-
 t/t2700-refresh.sh        |   57 ++++++++-
 12 files changed, 368 insertions(+), 200 deletions(-)

                                 -+-

The following changes since commit 8ae7dc9d485fe5f3fee02ead7f25133be7321614:
  Karl Hasselström (1):
        Emacs mode: It's possible to edit unapplied patches now

are available in the git repository at:

  git://repo.or.cz/stgit/kha.git experimental

Karl Hasselström (7):
      Disable patchlog test for "stg new"
      Convert "stg new" to the new infrastructure
      Refactor --author/--committer options
      Let "stg new" support more message options
      Emacs mode: use "stg new --file"
      Convert "stg delete" to the new infrastructure
      Emacs mode: delete patches

 contrib/stgit.el         |   20 +++++----
 stgit/commands/common.py |   33 +++++----------
 stgit/commands/delete.py |   72 +++++++++++-----------------------
 stgit/commands/edit.py   |   25 +----------
 stgit/commands/new.py    |   97 +++++++++++++++++++++++++--------------------
 stgit/lib/git.py         |    8 ++--
 stgit/utils.py           |   50 +++++++++++++++++++++++
 t/t1400-patch-history.sh |    2 -
 t/t1600-delete-one.sh    |    8 ++--
 9 files changed, 160 insertions(+), 155 deletions(-)

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

[StGit PATCH 0/5] Convert "stg new" to the new infrastructure

[Karl Hasselström]

---

Karl Hasselström (5):
      Emacs mode: use "stg new --file"
      Let "stg new" support more message options
      Refactor --author/--committer options
      Convert "stg new" to the new infrastructure
      Disable patchlog test for "stg new"


 contrib/stgit.el         |   10 +----
 stgit/commands/common.py |   33 +++++-----------
 stgit/commands/edit.py   |   25 +-----------
 stgit/commands/new.py    |   97 ++++++++++++++++++++++++++--------------------
 stgit/lib/git.py         |    8 ++--
 stgit/utils.py           |   50 ++++++++++++++++++++++++
 t/t1400-patch-history.sh |    2 -
 7 files changed, 124 insertions(+), 101 deletions(-)

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

[StGit PATCH 1/5] Disable patchlog test for "stg new"

[Karl Hasselström]

This will be broken by the "stg new" rewrite, so stop testing it.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

I'm doing this rather than fixing the log since I hope to actually
create "stg undo" pretty soon, and that'll involve a new internal log
format. But I won't graduate this to kha/safe until it's resolved
either way.

 t/t1400-patch-history.sh |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)


diff --git a/t/t1400-patch-history.sh b/t/t1400-patch-history.sh
index 879b1a5..a693e75 100755
--- a/t/t1400-patch-history.sh
+++ b/t/t1400-patch-history.sh
@@ -35,9 +35,7 @@ test_expect_success \
 test_expect_success \
 	'Check the "new" and "refresh" logs' \
 	'
-	stg log --full foo | grep -q -e "^new" &&
 	stg log --full foo | grep -q -e "^refresh" &&
-	stg log --full | grep -q -e "^new" &&
 	stg log --full | grep -q -e "^refresh"
 	'
 

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

[Karl Hasselström]

This results in considerable code expansion, which is a sure sign that
something needs to be abstracted away -- but to keep things simple,
those transformations come as separate patches.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 stgit/commands/new.py |   85 ++++++++++++++++++++++++++++++++-----------------
 stgit/lib/git.py      |    8 ++---
 2 files changed, 60 insertions(+), 33 deletions(-)


diff --git a/stgit/commands/new.py b/stgit/commands/new.py
index 6a8f086..dd9f93e 100644
--- a/stgit/commands/new.py
+++ b/stgit/commands/new.py
@@ -16,13 +16,11 @@ along with this program; if not, write to the Free Software
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 """
 
-import sys, os
-from optparse import OptionParser, make_option
-
-from stgit.commands.common import *
-from stgit.utils import *
-from stgit import stack, git
+from optparse import make_option
 
+from stgit import utils
+from stgit.commands import common
+from stgit.lib import git as gitlib, transaction
 
 help = 'create a new patch and make it the topmost one'
 usage = """%prog [options] [name]
@@ -38,12 +36,9 @@ this.
 If no name is given for the new patch, one is generated from the first
 line of the commit message."""
 
-directory = DirectoryGotoToplevel()
+directory = common.DirectoryHasRepositoryLib()
 options = [make_option('-m', '--message',
                        help = 'use MESSAGE as the patch description'),
-           make_option('-s', '--showpatch',
-                       help = 'show the patch content in the editor buffer',
-                       action = 'store_true'),
            make_option('-a', '--author', metavar = '"NAME <EMAIL>"',
                        help = 'use "NAME <EMAIL>" as the author details'),
            make_option('--authname',
@@ -56,30 +51,62 @@ options = [make_option('-m', '--message',
                        help = 'use COMMNAME as the committer name'),
            make_option('--commemail',
                        help = 'use COMMEMAIL as the committer e-mail')
-           ] + make_sign_options()
-
+           ] + utils.make_sign_options()
 
 def func(parser, options, args):
-    """Creates a new patch
-    """
+    """Create a new patch."""
+    stack = directory.repository.current_stack
+    if stack.repository.default_index.conflicts():
+        raise common.CmdException(
+            'Cannot create a new patch -- resolve conflicts first')
+
+    # Choose a name for the new patch -- or None, which means make one
+    # up later when we've gotten hold of the commit message.
     if len(args) == 0:
-        name = None # autogenerate a name
+        name = None
     elif len(args) == 1:
         name = args[0]
+        if stack.patches.exists(name):
+            raise common.CmdException('%s: patch already exists' % name)
     else:
         parser.error('incorrect number of arguments')
 
-    check_conflicts()
-    check_head_top_equal(crt_series)
-
-    if options.author:
-        options.authname, options.authemail = name_email(options.author)
-
-    crt_series.new_patch(name, message = options.message,
-                         show_patch = options.showpatch,
-                         author_name = options.authname,
-                         author_email = options.authemail,
-                         author_date = options.authdate,
-                         committer_name = options.commname,
-                         committer_email = options.commemail,
-                         sign_str = options.sign_str)
+    head = directory.repository.refs.get(directory.repository.head)
+    cd = gitlib.Commitdata(tree = head.data.tree, parents = [head],
+                           message = '')
+
+    # Set patch commit message from commandline.
+    if options.message != None:
+        cd = cd.set_message(options.message)
+
+    # Specify 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))
+
+    # Let user edit the commit message manually.
+    if not options.message:
+        cd = cd.set_message(utils.edit_string(cd.message, '.stgit-new.txt'))
+    if name == None:
+        name = utils.make_patch_name(cd.message,
+                                     lambda name: stack.patches.exists(name))
+
+    # Write the new patch.
+    iw = stack.repository.default_iw
+    trans = transaction.StackTransaction(stack, 'stg new')
+    trans.patches[name] = stack.repository.commit(cd)
+    trans.applied.append(name)
+    return trans.run()
diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 50dc4f1..6ee8a71 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -24,13 +24,13 @@ class NoValue(object):
     pass
 
 def make_defaults(defaults):
-    def d(val, attr):
+    def d(val, attr, default_fun = lambda: None):
         if val != NoValue:
             return val
         elif defaults != NoValue:
             return getattr(defaults, attr)
         else:
-            return None
+            return default_fun()
     return d
 
 class TimeZone(tzinfo, Repr):
@@ -161,8 +161,8 @@ class Commitdata(Repr):
         d = make_defaults(defaults)
         self.__tree = d(tree, 'tree')
         self.__parents = d(parents, 'parents')
-        self.__author = d(author, 'author')
-        self.__committer = d(committer, 'committer')
+        self.__author = d(author, 'author', Person.author)
+        self.__committer = d(committer, 'committer', Person.committer)
         self.__message = d(message, 'message')
     tree = property(lambda self: self.__tree)
     parents = property(lambda self: self.__parents)

[StGit PATCH 3/5] Refactor --author/--committer options

[Karl Hasselström]

This refactoring is specific to the new infrastructure, so only new
and edit use it currently, but other commands can start using it as
they are converted.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 stgit/commands/common.py |   33 ++++++++++--------------------
 stgit/commands/edit.py   |   25 +++--------------------
 stgit/commands/new.py    |   40 ++++++++++---------------------------
 stgit/utils.py           |   50 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 75 insertions(+), 73 deletions(-)


diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index 5a1952b..d6df813 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -266,30 +266,19 @@ def parse_patches(patch_args, patch_list, boundary = 0, ordered = False):
     return patches
 
 def name_email(address):
-    """Return a tuple consisting of the name and email parsed from a
-    standard 'name <email>' or 'email (name)' string
-    """
-    address = re.sub(r'[\\"]', r'\\\g<0>', address)
-    str_list = re.findall('^(.*)\s*<(.*)>\s*$', address)
-    if not str_list:
-        str_list = re.findall('^(.*)\s*\((.*)\)\s*$', address)
-        if not str_list:
-            raise CmdException('Incorrect "name <email>"/"email (name)"'
-                               ' string: %s' % address)
-        return ( str_list[0][1], str_list[0][0] )
-
-    return str_list[0]
+    p = parse_name_email(address)
+    if p:
+        return p
+    else:
+        raise CmdException('Incorrect "name <email>"/"email (name)" string: %s'
+                           % address)
 
 def name_email_date(address):
-    """Return a tuple consisting of the name, email and date parsed
-    from a 'name <email> date' string
-    """
-    address = re.sub(r'[\\"]', r'\\\g<0>', address)
-    str_list = re.findall('^(.*)\s*<(.*)>\s*(.*)\s*$', address)
-    if not str_list:
-        raise CmdException, 'Incorrect "name <email> date" string: %s' % address
-
-    return str_list[0]
+    p = parse_name_email_date(address)
+    if p:
+        return p
+    else:
+        raise CmdException('Incorrect "name <email> date" string: %s' % address)
 
 def address_or_alias(addr_str):
     """Return the address if it contains an e-mail address or look up
diff --git a/stgit/commands/edit.py b/stgit/commands/edit.py
index 7daf156..037425b 100644
--- a/stgit/commands/edit.py
+++ b/stgit/commands/edit.py
@@ -60,19 +60,8 @@ options = [make_option('-d', '--diff',
                        action = 'store_true'),
            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',
-                       help = 'replace the author name with AUTHNAME'),
-           make_option('--authemail',
-                       help = 'replace the author e-mail with AUTHEMAIL'),
-           make_option('--authdate',
-                       help = 'replace the author date with AUTHDATE'),
-           make_option('--commname',
-                       help = 'replace the committer name with COMMNAME'),
-           make_option('--commemail',
-                       help = 'replace the committer e-mail with COMMEMAIL')
            ] + (utils.make_sign_options() + utils.make_message_options()
+                + utils.make_author_committer_options()
                 + utils.make_diff_opts_option())
 
 def patch_diff(repository, cd, diff, diff_flags):
@@ -141,16 +130,8 @@ def func(parser, options, args):
                                                    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))
+    cd = (cd.set_author(options.author(cd.author))
+            .set_committer(options.committer(cd.committer)))
 
     # Add Signed-off-by: or similar.
     if options.sign_str != None:
diff --git a/stgit/commands/new.py b/stgit/commands/new.py
index dd9f93e..d44b8cc 100644
--- a/stgit/commands/new.py
+++ b/stgit/commands/new.py
@@ -39,19 +39,8 @@ line of the commit message."""
 directory = common.DirectoryHasRepositoryLib()
 options = [make_option('-m', '--message',
                        help = 'use MESSAGE as the patch description'),
-           make_option('-a', '--author', metavar = '"NAME <EMAIL>"',
-                       help = 'use "NAME <EMAIL>" as the author details'),
-           make_option('--authname',
-                       help = 'use AUTHNAME as the author name'),
-           make_option('--authemail',
-                       help = 'use AUTHEMAIL as the author e-mail'),
-           make_option('--authdate',
-                       help = 'use AUTHDATE as the author date'),
-           make_option('--commname',
-                       help = 'use COMMNAME as the committer name'),
-           make_option('--commemail',
-                       help = 'use COMMEMAIL as the committer e-mail')
-           ] + utils.make_sign_options()
+           ] + (utils.make_author_committer_options()
+                + utils.make_sign_options())
 
 def func(parser, options, args):
     """Create a new patch."""
@@ -72,30 +61,23 @@ def func(parser, options, args):
         parser.error('incorrect number of arguments')
 
     head = directory.repository.refs.get(directory.repository.head)
-    cd = gitlib.Commitdata(tree = head.data.tree, parents = [head],
-                           message = '')
+    cd = gitlib.Commitdata(
+        tree = head.data.tree, parents = [head], message = '',
+        author = gitlib.Person.author(), committer = gitlib.Person.committer())
 
     # Set patch commit message from commandline.
     if options.message != None:
         cd = cd.set_message(options.message)
 
-    # Specify 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))
+    # Modify author and committer data.
+    cd = (cd.set_author(options.author(cd.author))
+            .set_committer(options.committer(cd.committer)))
 
     # 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 = cd.set_message(
+            utils.add_sign_line(cd.message, options.sign_str,
+                                cd.committer.name, cd.committer.email))
 
     # Let user edit the commit message manually.
     if not options.message:
diff --git a/stgit/utils.py b/stgit/utils.py
index b75c3b4..947f747 100644
--- a/stgit/utils.py
+++ b/stgit/utils.py
@@ -322,6 +322,56 @@ def make_diff_opts_option():
         type = 'string', metavar = 'OPTIONS',
         help = 'extra options to pass to "git diff"')]
 
+def parse_name_email(address):
+    """Return a tuple consisting of the name and email parsed from a
+    standard 'name <email>' or 'email (name)' string."""
+    address = re.sub(r'[\\"]', r'\\\g<0>', address)
+    str_list = re.findall(r'^(.*)\s*<(.*)>\s*$', address)
+    if not str_list:
+        str_list = re.findall(r'^(.*)\s*\((.*)\)\s*$', address)
+        if not str_list:
+            return None
+        return (str_list[0][1], str_list[0][0])
+    return str_list[0]
+
+def parse_name_email_date(address):
+    """Return a tuple consisting of the name, email and date parsed
+    from a 'name <email> date' string."""
+    address = re.sub(r'[\\"]', r'\\\g<0>', address)
+    str_list = re.findall('^(.*)\s*<(.*)>\s*(.*)\s*$', address)
+    if not str_list:
+        return None
+    return str_list[0]
+
+def make_person_options(person, short):
+    """Sets options.<person> to a function that modifies a Person
+    according to the commandline options."""
+    def short_callback(option, opt_str, value, parser, field):
+        f = getattr(parser.values, person)
+        setattr(parser.values, person,
+                lambda p: getattr(f(p), 'set_' + field)(value))
+    def full_callback(option, opt_str, value, parser):
+        ne = parse_name_email(value)
+        if not ne:
+            raise optparse.OptionValueError(
+                'Bad %s specification: %r' % (opt_str, value))
+        name, email = ne
+        short_callback(option, opt_str, name, parser, 'name')
+        short_callback(option, opt_str, email, parser, 'email')
+    return ([optparse.make_option(
+                '--%s' % person, metavar = '"NAME <EMAIL>"', type = 'string',
+                action = 'callback', callback = full_callback, dest = person,
+                default = lambda p: p, help = 'set the %s details' % person)]
+            + [optparse.make_option(
+                '--%s%s' % (short, f), metavar = f.upper(), type = 'string',
+                action = 'callback', callback = short_callback, dest = person,
+                callback_args = (f,), help = 'set the %s %s' % (person, f))
+               for f in ['name', 'email', 'date']])
+
+def make_author_committer_options():
+    return (make_person_options('author', 'auth')
+            + make_person_options('committer', 'comm'))
+
 # Exit codes.
 STGIT_SUCCESS = 0        # everything's OK
 STGIT_GENERAL_ERROR = 1  # seems to be non-command-specific error

[StGit PATCH 4/5] Let "stg new" support more message options

[Karl Hasselström]

Let "stg new" support --file, and --save-template in addition to
--message. This is useful for scripting.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 stgit/commands/new.py |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)


diff --git a/stgit/commands/new.py b/stgit/commands/new.py
index d44b8cc..43414f5 100644
--- a/stgit/commands/new.py
+++ b/stgit/commands/new.py
@@ -37,10 +37,8 @@ If no name is given for the new patch, one is generated from the first
 line of the commit message."""
 
 directory = common.DirectoryHasRepositoryLib()
-options = [make_option('-m', '--message',
-                       help = 'use MESSAGE as the patch description'),
-           ] + (utils.make_author_committer_options()
-                + utils.make_sign_options())
+options = (utils.make_author_committer_options()
+           + utils.make_message_options() + utils.make_sign_options())
 
 def func(parser, options, args):
     """Create a new patch."""
@@ -79,6 +77,10 @@ def func(parser, options, args):
             utils.add_sign_line(cd.message, options.sign_str,
                                 cd.committer.name, cd.committer.email))
 
+    if options.save_template:
+        options.save_template(cd.message)
+        return utils.STGIT_SUCCESS
+
     # Let user edit the commit message manually.
     if not options.message:
         cd = cd.set_message(utils.edit_string(cd.message, '.stgit-new.txt'))

[StGit PATCH 5/5] Emacs mode: use "stg new --file"

[Karl Hasselström]

Creating a new patch is a great deal easier now that "stg new" has a
--file flag.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

David, will you sanity-check this? I don't really speak elisp, so I
might have done something insane without knowing it. But it does seem
to work. :-)

 contrib/stgit.el |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)


diff --git a/contrib/stgit.el b/contrib/stgit.el
index bef41c7..30c1cd1 100644
--- a/contrib/stgit.el
+++ b/contrib/stgit.el
@@ -316,16 +316,10 @@ Commands:
 
 (defun stgit-confirm-new ()
   (interactive)
-  (let ((file (make-temp-file "stgit-edit-"))
-        (patch (stgit-create-patch-name
-                (buffer-substring (point-min)
-                                  (save-excursion (goto-char (point-min))
-                                                  (end-of-line)
-                                                  (point))))))
+  (let ((file (make-temp-file "stgit-edit-")))
     (write-region (point-min) (point-max) file)
     (stgit-capture-output nil
-      (stgit-run "new" "-m" "placeholder" patch)
-      (stgit-run "edit" "-f" file patch))
+      (stgit-run "new" "-f" file))
     (with-current-buffer log-edit-parent-buffer
       (stgit-refresh))))
 

[StGit PATCH 0/2] Convert "stg delete" to the new infrastructure

[Karl Hasselström]

---

Karl Hasselström (2):
      Emacs mode: delete patches
      Convert "stg delete" to the new infrastructure


 contrib/stgit.el         |   10 ++++++
 stgit/commands/delete.py |   72 +++++++++++++++-------------------------------
 t/t1600-delete-one.sh    |    8 +++--
 3 files changed, 36 insertions(+), 54 deletions(-)

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

[StGit PATCH 1/2] Convert "stg delete" to the new infrastructure

[Karl Hasselström]

In the process, it gains the ability to delete any applied patch (not
just the topmost one, like before), even when deleting patches from
another branch. (However, when deleting patches on another branch, we
obviously can't represent a conflict in the index and worktree, so any
conflicts will make the operation abort.)

One of the t1600 subtests made sure that we couldn't delete
non-topmost patches, and had to be corrected.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

Unlike the "stg new" conversion, this resulted in a sizeable reduction
of lines of code, and of code complexity.

 stgit/commands/delete.py |   72 +++++++++++++++-------------------------------
 t/t1600-delete-one.sh    |    8 +++--
 2 files changed, 27 insertions(+), 53 deletions(-)


diff --git a/stgit/commands/delete.py b/stgit/commands/delete.py
index 1696cb9..106fbd2 100644
--- a/stgit/commands/delete.py
+++ b/stgit/commands/delete.py
@@ -16,67 +16,41 @@ along with this program; if not, write to the Free Software
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 """
 
-import sys, os
-from optparse import OptionParser, make_option
-
-from stgit.commands.common import *
-from stgit.utils import *
-from stgit.out import *
-from stgit import stack, git
+from optparse import make_option
 
+from stgit.commands import common
+from stgit.lib import transaction
 
 help = 'delete patches'
 usage = """%prog [options] <patch1> [<patch2>] [<patch3>..<patch4>]
 
-Delete the patches passed as arguments. If an applied patch is to be
-deleted, all other patches applied on top of it must be deleted too,
-and they must be explicitly specified, since this command will not try
-to delete a patch unless you explicitly ask it to. If any applied
-patches are deleted, they are popped from the stack.
+Delete the patches passed as arguments.
 
 Note that the 'delete' operation is irreversible."""
 
-directory = DirectoryGotoToplevel()
+directory = common.DirectoryHasRepositoryLib()
 options = [make_option('-b', '--branch',
                        help = 'use BRANCH instead of the default one')]
 
 def func(parser, options, args):
-    """Deletes one or more patches.
-    """
-    applied_patches = crt_series.get_applied()
-    unapplied_patches = crt_series.get_unapplied()
-    all_patches = applied_patches + unapplied_patches
-
+    """Delete one or more patches."""
+    if options.branch:
+        stack = directory.repository.get_stack(options.branch)
+        iw = None # can't use index/workdir to manipulate another branch
+    else:
+        stack = directory.repository.current_stack
+        iw = stack.repository.default_iw
     if args:
-        patches = parse_patches(args, all_patches, len(applied_patches))
+        patches = set(common.parse_patches(
+                args, (list(stack.patchorder.applied)
+                       + list(stack.patchorder.unapplied))))
     else:
         parser.error('No patches specified')
-
-    applied = []
-
-    # find the applied patches to be deleted. We can only delete
-    # consecutive patches in the applied range
-    for patch in applied_patches[::-1]:
-        if patch in patches:
-            applied.append(patch)
-            patches.remove(patch)
-        else:
-            break
-
-    # any applied patches to be deleted but not in consecutive order?
-    for patch in patches:
-        if patch in applied_patches:
-            raise CmdException, 'Cannot delete the applied patch "%s"' % patch
-
-    if applied and not options.branch:
-        check_local_changes()
-        check_conflicts()
-        check_head_top_equal(crt_series)
-
-    # delete the patches
-    for patch in applied + patches:
-        crt_series.delete_patch(patch)
-        out.info('Patch "%s" successfully deleted' % patch)
-
-    if not options.branch:
-        print_crt_patch(crt_series)
+    trans = transaction.StackTransaction(stack, 'stg delete')
+    try:
+        to_push = trans.delete_patches(lambda pn: pn in patches)
+        for pn in to_push:
+            trans.push_patch(pn, iw)
+    except transaction.TransactionHalted:
+        pass
+    return trans.run(iw)
diff --git a/t/t1600-delete-one.sh b/t/t1600-delete-one.sh
index 3052b3a..c3451d8 100755
--- a/t/t1600-delete-one.sh
+++ b/t/t1600-delete-one.sh
@@ -77,8 +77,8 @@ test_expect_success \
     'Try to delete a non-topmost applied patch' \
     '
     [ $(stg applied | wc -l) -eq 2 ] &&
-    ! stg delete foo &&
-    [ $(stg applied | wc -l) -eq 2 ]
+    stg delete foo &&
+    [ $(stg applied | wc -l) -eq 1 ]
     '
 
 test_expect_success \
@@ -99,10 +99,10 @@ test_expect_success \
 test_expect_success \
     'Delete a patch in another branch' \
     '
-    [ $(stg applied | wc -l) -eq 3 ] &&
+    [ $(stg applied | wc -l) -eq 2 ] &&
     [ $(stg applied -b br | wc -l) -eq 1 ] &&
     stg delete -b br baz &&
-    [ $(stg applied | wc -l) -eq 3 ] &&
+    [ $(stg applied | wc -l) -eq 2 ] &&
     [ $(stg applied -b br | wc -l) -eq 0 ]
     '
 

[StGit PATCH 2/2] Emacs mode: delete patches

[Karl Hasselström]

Teach the emacs mode to delete patches.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

David, could you have a look at this as well? In addition to me being
elisp challenged, there are the following issues:

  * Is "d" a reasonable binding? Any better suggestion?

  * Currently, this command requires you to mark one or more patches
    before deleting. This is convenient when deleting more than one
    patch, but one could argue that it should be possible to delete
    the patch at point without having to select it.

  * We should probably ask for confirmation before deleting.

 contrib/stgit.el |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)


diff --git a/contrib/stgit.el b/contrib/stgit.el
index 30c1cd1..7468fcd 100644
--- a/contrib/stgit.el
+++ b/contrib/stgit.el
@@ -150,7 +150,8 @@ Argument DIR is the repository path."
   (define-key stgit-mode-map "<"   'stgit-pop-next)
   (define-key stgit-mode-map "P"   'stgit-push-or-pop)
   (define-key stgit-mode-map "G"   'stgit-goto)
-  (define-key stgit-mode-map "="   'stgit-show))
+  (define-key stgit-mode-map "="   'stgit-show)
+  (define-key stgit-mode-map "d"   'stgit-delete))
 
 (defun stgit-mode ()
   "Major mode for interacting with StGit.
@@ -341,6 +342,13 @@ Commands:
            (substring patch 0 20))
           (t patch))))
 
+(defun stgit-delete (patch-names)
+  "Delete the named patches"
+  (interactive (list (stgit-marked-patches)))
+  (stgit-capture-output nil
+    (apply 'stgit-run "delete" patch-names))
+  (stgit-refresh))
+
 (defun stgit-coalesce (patch-names)
   "Run stg coalesce on the named patches"
   (interactive (list (stgit-marked-patches)))

Re: [StGit PATCH 5/5] Emacs mode: use "stg new --file"

[David Kågedal]

Karl Hasselström <kha@treskal.com> writes:

> Creating a new patch is a great deal easier now that "stg new" has a
> --file flag.
>
> Signed-off-by: Karl Hasselström <kha@treskal.com>
>
> ---
>
> David, will you sanity-check this? I don't really speak elisp, so I
> might have done something insane without knowing it. But it does seem
> to work. :-)

Looks sane to me.

>  contrib/stgit.el |   10 ++--------
>  1 files changed, 2 insertions(+), 8 deletions(-)
>
>
> diff --git a/contrib/stgit.el b/contrib/stgit.el
> index bef41c7..30c1cd1 100644
> --- a/contrib/stgit.el
> +++ b/contrib/stgit.el
> @@ -316,16 +316,10 @@ Commands:
>  
>  (defun stgit-confirm-new ()
>    (interactive)
> -  (let ((file (make-temp-file "stgit-edit-"))
> -        (patch (stgit-create-patch-name
> -                (buffer-substring (point-min)
> -                                  (save-excursion (goto-char (point-min))
> -                                                  (end-of-line)
> -                                                  (point))))))
> +  (let ((file (make-temp-file "stgit-edit-")))
>      (write-region (point-min) (point-max) file)
>      (stgit-capture-output nil
> -      (stgit-run "new" "-m" "placeholder" patch)
> -      (stgit-run "edit" "-f" file patch))
> +      (stgit-run "new" "-f" file))
>      (with-current-buffer log-edit-parent-buffer
>        (stgit-refresh))))
>  
>

-- 
David Kågedal

Re: [StGit PATCH 2/2] Emacs mode: delete patches

[David Kågedal]

Karl Hasselström <kha@treskal.com> writes:

> Teach the emacs mode to delete patches.
>
> Signed-off-by: Karl Hasselström <kha@treskal.com>
>
> ---
>
> David, could you have a look at this as well? In addition to me being
> elisp challenged, there are the following issues:
>
>   * Is "d" a reasonable binding? Any better suggestion?

We could reserve "d" for moving a patch "down", maybe.  The more
destructive commands could be on less accessible keys. Maybe "D" or
"C-d".

>   * Currently, this command requires you to mark one or more patches
>     before deleting. This is convenient when deleting more than one
>     patch, but one could argue that it should be possible to delete
>     the patch at point without having to select it.

You need something like this:

--- a/contrib/stgit.el
+++ b/contrib/stgit.el
@@ -187,6 +187,15 @@ Commands:
         (match-string-no-properties 1)
       nil)))
 
+(defun stgit-selected-patches ()
+  "Return the names of the marked patches, or the patch on the current line."
+  (if stgit-marked-patches
+      (stgit-marked-patches)
+    (let ((patch (stgit-patch-at-point)))
+      (if patch
+          (list patch)
+        '()))))
+
 (defun stgit-goto-patch (patch)
   "Move point to the line containing PATCH"
   (let ((p (point)))

>   * We should probably ask for confirmation before deleting.

Absolutely. Something like this (untested):

(defun stgit-delete (patch-names)
  "Delete the named patches"
  (interactive (list (stgit-selected-patches)))
  (if (zerop (length patch-names))
      (error "No patches to delete")
    (when (yes-or-no-p (format "Really delete %d patches? "
                               (length patch-names)))
      (stgit-capture-output nil
        (apply 'stgit-run "delete" patch-names))
      (stgit-refresh))


-- 
David Kågedal

Re: [StGit PATCH 5/5] Emacs mode: use "stg new --file"

[Karl Hasselström]

On 2008-02-11 10:25:44 +0100, David Kågedal wrote:

> Karl Hasselström <kha@treskal.com> writes:
>
> > David, will you sanity-check this? I don't really speak elisp, so
> > I might have done something insane without knowing it. But it does
> > seem to work. :-)
>
> Looks sane to me.

Thanks.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [StGit PATCH 2/2] Emacs mode: delete patches

[Karl Hasselström]

On 2008-02-11 10:42:22 +0100, David Kågedal wrote:

> Karl Hasselström <kha@treskal.com> writes:
>
> > David, could you have a look at this as well? In addition to me
> > being elisp challenged, there are the following issues:
> >
> >   * Is "d" a reasonable binding? Any better suggestion?
>
> We could reserve "d" for moving a patch "down", maybe. The more
> destructive commands could be on less accessible keys. Maybe "D" or
> "C-d".

I'll take "D" then, since control bindings are more likely to collide
with existing bindings (as is already the case with C-r for stg
repair).

> >   * Currently, this command requires you to mark one or more
> >     patches before deleting. This is convenient when deleting more
> >     than one patch, but one could argue that it should be possible
> >     to delete the patch at point without having to select it.
>
> You need something like this:

OK, thanks. I'll whip up a proper patch tonight -- unless you prefer
to do it yourself?

> >   * We should probably ask for confirmation before deleting.
>
> Absolutely. Something like this (untested):

Thanks again.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [StGit PATCH 2/2] Emacs mode: delete patches

[David Kågedal]

Karl Hasselström <kha@treskal.com> writes:

> On 2008-02-11 10:42:22 +0100, David Kågedal wrote:
>
>> Karl Hasselström <kha@treskal.com> writes:
>>
>> > David, could you have a look at this as well? In addition to me
>> > being elisp challenged, there are the following issues:
>> >
>> >   * Is "d" a reasonable binding? Any better suggestion?
>>
>> We could reserve "d" for moving a patch "down", maybe. The more
>> destructive commands could be on less accessible keys. Maybe "D" or
>> "C-d".
>
> I'll take "D" then, since control bindings are more likely to collide
> with existing bindings (as is already the case with C-r for stg
> repair).

True, maybe we should move the repair binding as well.

>> >   * Currently, this command requires you to mark one or more
>> >     patches before deleting. This is convenient when deleting more
>> >     than one patch, but one could argue that it should be possible
>> >     to delete the patch at point without having to select it.
>>
>> You need something like this:
>
> OK, thanks. I'll whip up a proper patch tonight -- unless you prefer
> to do it yourself?

Be my guest.

-- 
David Kågedal

[StGit PATCH 1/2] Emacs mode: change "stg repair" binding

[Karl Hasselström]

It used to be C-r, but that's commonly used for reverse searching, so
use R instead.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 contrib/stgit.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/contrib/stgit.el b/contrib/stgit.el
index e8bbb2c..aa54772 100644
--- a/contrib/stgit.el
+++ b/contrib/stgit.el
@@ -143,7 +143,7 @@ Argument DIR is the repository path."
   (define-key stgit-mode-map "e"   'stgit-edit)
   (define-key stgit-mode-map "c"   'stgit-coalesce)
   (define-key stgit-mode-map "N"   'stgit-new)
-  (define-key stgit-mode-map "\C-r"   'stgit-repair)
+  (define-key stgit-mode-map "R"   'stgit-repair)
   (define-key stgit-mode-map "C"   'stgit-commit)
   (define-key stgit-mode-map "U"   'stgit-uncommit)
   (define-key stgit-mode-map ">"   'stgit-push-next)

[StGit PATCH v2 2/2] Emacs mode: delete patches

[Karl Hasselström]

Teach the emacs mode to delete patches. It will delete the selected
patches, or, if no patches are selected, the patch at point (and if
there is no patch at point, complain). Before deleting, it will ask
for confirmation.

Thanks to David Kågedal <davidk@lysator.liu.se> for lots of elisp
help.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 contrib/stgit.el |   23 ++++++++++++++++++++++-
 1 files changed, 22 insertions(+), 1 deletions(-)


diff --git a/contrib/stgit.el b/contrib/stgit.el
index 386d46e..1036cf9 100644
--- a/contrib/stgit.el
+++ b/contrib/stgit.el
@@ -150,7 +150,8 @@ Argument DIR is the repository path."
   (define-key stgit-mode-map "<"   'stgit-pop-next)
   (define-key stgit-mode-map "P"   'stgit-push-or-pop)
   (define-key stgit-mode-map "G"   'stgit-goto)
-  (define-key stgit-mode-map "="   'stgit-show))
+  (define-key stgit-mode-map "="   'stgit-show)
+  (define-key stgit-mode-map "D"   'stgit-delete))
 
 (defun stgit-mode ()
   "Major mode for interacting with StGit.
@@ -187,6 +188,15 @@ Commands:
         (match-string-no-properties 1)
       nil)))
 
+(defun stgit-patches-marked-or-at-point ()
+  "Return the names of the marked patches, or the patch on the current line."
+  (if stgit-marked-patches
+      (stgit-marked-patches)
+    (let ((patch (stgit-patch-at-point)))
+      (if patch
+          (list patch)
+        '()))))
+
 (defun stgit-goto-patch (patch)
   "Move point to the line containing PATCH"
   (let ((p (point)))
@@ -341,6 +351,17 @@ Commands:
            (substring patch 0 20))
           (t patch))))
 
+(defun stgit-delete (patch-names)
+  "Delete the named patches"
+  (interactive (list (stgit-patches-marked-or-at-point)))
+  (if (zerop (length patch-names))
+      (error "No patches to delete")
+    (when (yes-or-no-p (format "Really delete %d patches? "
+                               (length patch-names)))
+      (stgit-capture-output nil
+        (apply 'stgit-run "delete" patch-names))
+      (stgit-refresh))))
+
 (defun stgit-coalesce (patch-names)
   "Run stg coalesce on the named patches"
   (interactive (list (stgit-marked-patches)))

Re: StGit: kha/safe and kha/experimental updated

[Catalin Marinas]

On 10/02/2008, Karl Hasselström <kha@treskal.com> wrote:
> Both branches updated. I've promoted some experimental patches to
> safe, and done some work on new and delete (will post the patches in
> reply to this mail).

Thanks, I merged them last night. I now have to spend some time to
understand StGIT as it's moving too fast :-). I'll try to put back
some of the things I use like the automatic invocation of the merge
tool during conflicts etc.

-- 
Catalin

Subject: [PATCH] fix stg edit command

[Peter Oberndorfer]

The comparison of two Commitdata objects returned False.
Signed-off-by: Peter Oberndorfer <kumbayo84@arcor.de>
---

On Sonntag 10 Februar 2008, Karl Hasselström wrote:
> This refactoring is specific to the new infrastructure, so only new
> and edit use it currently, but other commands can start using it as
> they are converted.
> 
> Signed-off-by: Karl Hasselström <kha@treskal.com>

While testing my editor searching ordering patch i found that
this patch(Refactor --author/--committer options) seems to break
"stg edit" (without arguments) starting a interactive editor for me.
When i issue "stg edit" it silently does nothing.

It seems the following comparison does not return True

># Let user edit the patch manually.
>if cd == orig_cd or options.edit:

I can work around this by adding a comparison function to Commitdata
but maybe __eq__ or __ne__ should be used instead(prevent similar bugs caused
by == comparison)?
I would guess the initial cause for this is that the following line which unconditionally replaces cd.

> -    # Specify 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))
> +    # Modify author and committer data.
> +    cd = (cd.set_author(options.author(cd.author))
> +            .set_committer(options.committer(cd.committer)))

So another way to fix this might be, to not overwrite cd unconditionally.

Greetings Peter

 stgit/commands/edit.py |    2 +-
 stgit/lib/git.py       |    8 ++++++++
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/stgit/commands/edit.py b/stgit/commands/edit.py
index 037425b..9439fb6 100644
--- a/stgit/commands/edit.py
+++ b/stgit/commands/edit.py
@@ -146,7 +146,7 @@ def func(parser, options, args):
         return utils.STGIT_SUCCESS
 
     # Let user edit the patch manually.
-    if cd == orig_cd or options.edit:
+    if cd.is_same(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(
diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 6ee8a71..b62d14c 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -202,6 +202,14 @@ class Commitdata(Repr):
         return ('Commitdata<tree: %s, parents: %s, author: %s,'
                 ' committer: %s, message: "%s">'
                 ) % (tree, parents, self.author, self.committer, self.message)
+
+    def is_same(self, other):
+        return (self.__tree == other.__tree and
+                self.__parents == other.__parents and
+                self.__author == other.__author and
+                self.__committer == other.__committer and
+                self.__message == other.__message)
+
     @classmethod
     def parse(cls, repository, s):
         cd = cls()
-- 
1.5.4.rc3

Re: Subject: [PATCH] fix stg edit command

[Karl Hasselström]

On 2008-02-12 23:05:05 +0100, Peter Oberndorfer wrote:

> While testing my editor searching ordering patch i found that this
> patch(Refactor --author/--committer options) seems to break "stg
> edit" (without arguments) starting a interactive editor for me. When
> i issue "stg edit" it silently does nothing.

Thanks for the report. And yes, as far as I can tell your analysis is
spot on. In the initial patch I remember being careful to not replace
cd unless it was actually changed, but obviously I got sloppy after
that. :-(

> It seems the following comparison does not return True
>
> > # Let user edit the patch manually.
> > if cd == orig_cd or options.edit:
>
> I can work around this by adding a comparison function to Commitdata
> but maybe __eq__ or __ne__ should be used instead(prevent similar
> bugs caused by == comparison)?
>
> +    def is_same(self, other):
> +        return (self.__tree == other.__tree and
> +                self.__parents == other.__parents and
> +                self.__author == other.__author and
> +                self.__committer == other.__committer and
> +                self.__message == other.__message)

Yes, you'd definitely want the common operators to work. But I usually
implement __cmp__ instead of __eq__ and __ne__ -- that gives you all
of <, <=, =, !=, >=, and > for the price of a single method. And it's
usually possible to define it rather simply in terms of cmp() with
tuple arguments, like this:

    def __cmp__(self, other):
        return cmp((self.__tree, self.__parents, self.__author,
                    self.__committer, self.__message),
                   (other.__tree, other.__parents, other.__author,
                    other.__committer, other.__message))

This sidesteps the great problem of cmp -- having to remember when to
return 1 and when to return -1.

> So another way to fix this might be, to not overwrite cd
> unconditionally.

Yes, this is what we want -- if the user gives --author, we shouldn't
open the interactive editor even if the given author is the same as
the patch already had.

Updated patch on the way.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

[StGit PATCH] Refactor --author/--committer options

[Karl Hasselström]

This refactoring is specific to the new infrastructure, so only new
and edit use it currently, but other commands can start using it as
they are converted.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

On 2008-02-12 23:47:24 +0100, Karl Hasselström wrote:

> On 2008-02-12 23:05:05 +0100, Peter Oberndorfer wrote:
>
> > So another way to fix this might be, to not overwrite cd
> > unconditionally.
>
> Yes, this is what we want -- if the user gives --author, we
> shouldn't open the interactive editor even if the given author is
> the same as the patch already had.
>
> Updated patch on the way.

And here it is. The relevant piece of code now reads

    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)

This works because the default value for options.author and
options.committer -- used when no commandline options are given -- is
the identity function.

 stgit/commands/common.py |   33 ++++++++++--------------------
 stgit/commands/edit.py   |   26 ++++--------------------
 stgit/commands/new.py    |   40 ++++++++++---------------------------
 stgit/utils.py           |   50 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 76 insertions(+), 73 deletions(-)


diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index 5a1952b..d6df813 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -266,30 +266,19 @@ def parse_patches(patch_args, patch_list, boundary = 0, ordered = False):
     return patches
 
 def name_email(address):
-    """Return a tuple consisting of the name and email parsed from a
-    standard 'name <email>' or 'email (name)' string
-    """
-    address = re.sub(r'[\\"]', r'\\\g<0>', address)
-    str_list = re.findall('^(.*)\s*<(.*)>\s*$', address)
-    if not str_list:
-        str_list = re.findall('^(.*)\s*\((.*)\)\s*$', address)
-        if not str_list:
-            raise CmdException('Incorrect "name <email>"/"email (name)"'
-                               ' string: %s' % address)
-        return ( str_list[0][1], str_list[0][0] )
-
-    return str_list[0]
+    p = parse_name_email(address)
+    if p:
+        return p
+    else:
+        raise CmdException('Incorrect "name <email>"/"email (name)" string: %s'
+                           % address)
 
 def name_email_date(address):
-    """Return a tuple consisting of the name, email and date parsed
-    from a 'name <email> date' string
-    """
-    address = re.sub(r'[\\"]', r'\\\g<0>', address)
-    str_list = re.findall('^(.*)\s*<(.*)>\s*(.*)\s*$', address)
-    if not str_list:
-        raise CmdException, 'Incorrect "name <email> date" string: %s' % address
-
-    return str_list[0]
+    p = parse_name_email_date(address)
+    if p:
+        return p
+    else:
+        raise CmdException('Incorrect "name <email> date" string: %s' % address)
 
 def address_or_alias(addr_str):
     """Return the address if it contains an e-mail address or look up
diff --git a/stgit/commands/edit.py b/stgit/commands/edit.py
index 7daf156..436f7ea 100644
--- a/stgit/commands/edit.py
+++ b/stgit/commands/edit.py
@@ -60,19 +60,8 @@ options = [make_option('-d', '--diff',
                        action = 'store_true'),
            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',
-                       help = 'replace the author name with AUTHNAME'),
-           make_option('--authemail',
-                       help = 'replace the author e-mail with AUTHEMAIL'),
-           make_option('--authdate',
-                       help = 'replace the author date with AUTHDATE'),
-           make_option('--commname',
-                       help = 'replace the committer name with COMMNAME'),
-           make_option('--commemail',
-                       help = 'replace the committer e-mail with COMMEMAIL')
            ] + (utils.make_sign_options() + utils.make_message_options()
+                + utils.make_author_committer_options()
                 + utils.make_diff_opts_option())
 
 def patch_diff(repository, cd, diff, diff_flags):
@@ -141,16 +130,9 @@ def func(parser, options, args):
                                                    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))
+    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:
diff --git a/stgit/commands/new.py b/stgit/commands/new.py
index dd9f93e..d44b8cc 100644
--- a/stgit/commands/new.py
+++ b/stgit/commands/new.py
@@ -39,19 +39,8 @@ line of the commit message."""
 directory = common.DirectoryHasRepositoryLib()
 options = [make_option('-m', '--message',
                        help = 'use MESSAGE as the patch description'),
-           make_option('-a', '--author', metavar = '"NAME <EMAIL>"',
-                       help = 'use "NAME <EMAIL>" as the author details'),
-           make_option('--authname',
-                       help = 'use AUTHNAME as the author name'),
-           make_option('--authemail',
-                       help = 'use AUTHEMAIL as the author e-mail'),
-           make_option('--authdate',
-                       help = 'use AUTHDATE as the author date'),
-           make_option('--commname',
-                       help = 'use COMMNAME as the committer name'),
-           make_option('--commemail',
-                       help = 'use COMMEMAIL as the committer e-mail')
-           ] + utils.make_sign_options()
+           ] + (utils.make_author_committer_options()
+                + utils.make_sign_options())
 
 def func(parser, options, args):
     """Create a new patch."""
@@ -72,30 +61,23 @@ def func(parser, options, args):
         parser.error('incorrect number of arguments')
 
     head = directory.repository.refs.get(directory.repository.head)
-    cd = gitlib.Commitdata(tree = head.data.tree, parents = [head],
-                           message = '')
+    cd = gitlib.Commitdata(
+        tree = head.data.tree, parents = [head], message = '',
+        author = gitlib.Person.author(), committer = gitlib.Person.committer())
 
     # Set patch commit message from commandline.
     if options.message != None:
         cd = cd.set_message(options.message)
 
-    # Specify 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))
+    # Modify author and committer data.
+    cd = (cd.set_author(options.author(cd.author))
+            .set_committer(options.committer(cd.committer)))
 
     # 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 = cd.set_message(
+            utils.add_sign_line(cd.message, options.sign_str,
+                                cd.committer.name, cd.committer.email))
 
     # Let user edit the commit message manually.
     if not options.message:
diff --git a/stgit/utils.py b/stgit/utils.py
index b75c3b4..947f747 100644
--- a/stgit/utils.py
+++ b/stgit/utils.py
@@ -322,6 +322,56 @@ def make_diff_opts_option():
         type = 'string', metavar = 'OPTIONS',
         help = 'extra options to pass to "git diff"')]
 
+def parse_name_email(address):
+    """Return a tuple consisting of the name and email parsed from a
+    standard 'name <email>' or 'email (name)' string."""
+    address = re.sub(r'[\\"]', r'\\\g<0>', address)
+    str_list = re.findall(r'^(.*)\s*<(.*)>\s*$', address)
+    if not str_list:
+        str_list = re.findall(r'^(.*)\s*\((.*)\)\s*$', address)
+        if not str_list:
+            return None
+        return (str_list[0][1], str_list[0][0])
+    return str_list[0]
+
+def parse_name_email_date(address):
+    """Return a tuple consisting of the name, email and date parsed
+    from a 'name <email> date' string."""
+    address = re.sub(r'[\\"]', r'\\\g<0>', address)
+    str_list = re.findall('^(.*)\s*<(.*)>\s*(.*)\s*$', address)
+    if not str_list:
+        return None
+    return str_list[0]
+
+def make_person_options(person, short):
+    """Sets options.<person> to a function that modifies a Person
+    according to the commandline options."""
+    def short_callback(option, opt_str, value, parser, field):
+        f = getattr(parser.values, person)
+        setattr(parser.values, person,
+                lambda p: getattr(f(p), 'set_' + field)(value))
+    def full_callback(option, opt_str, value, parser):
+        ne = parse_name_email(value)
+        if not ne:
+            raise optparse.OptionValueError(
+                'Bad %s specification: %r' % (opt_str, value))
+        name, email = ne
+        short_callback(option, opt_str, name, parser, 'name')
+        short_callback(option, opt_str, email, parser, 'email')
+    return ([optparse.make_option(
+                '--%s' % person, metavar = '"NAME <EMAIL>"', type = 'string',
+                action = 'callback', callback = full_callback, dest = person,
+                default = lambda p: p, help = 'set the %s details' % person)]
+            + [optparse.make_option(
+                '--%s%s' % (short, f), metavar = f.upper(), type = 'string',
+                action = 'callback', callback = short_callback, dest = person,
+                callback_args = (f,), help = 'set the %s %s' % (person, f))
+               for f in ['name', 'email', 'date']])
+
+def make_author_committer_options():
+    return (make_person_options('author', 'auth')
+            + make_person_options('committer', 'comm'))
+
 # Exit codes.
 STGIT_SUCCESS = 0        # everything's OK
 STGIT_GENERAL_ERROR = 1  # seems to be non-command-specific error


-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle