kha/safe and kha/experimental updated

kha/safe and kha/experimental updated

[Karl Hasselström]

I'll follow up with the three patch series in here that are new.

The following changes since commit cd885e085a697d5377956d5beb30e6030f224ccd:
  Peter Oberndorfer (1):
        replace "git repo-config" usage by "git config"

are available in the git repository at:

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

Karl Hasselström (10):
      Don't keep old committer when rewriting a commit
      Homogenize buffer names
      Remove unused default values
      Refactor --diff-opts handling
      Create index and worktree objects just once
      Wrap excessively long line
      Eliminate temp variable that's used just once
      Simplify editor selection logic
      Let the caller supply the diff text to diffstat()
      Don't clean away patches with conflicts

Pavel Roskin (1):
      Add test to ensure that "stg clean" preserves conflicting patches

 contrib/stgit.el           |    6 +++---
 stgit/commands/clean.py    |    7 +++++++
 stgit/commands/coalesce.py |    2 +-
 stgit/commands/commit.py   |    2 +-
 stgit/commands/common.py   |    3 ++-
 stgit/commands/diff.py     |   19 ++++++-------------
 stgit/commands/edit.py     |   16 ++++------------
 stgit/commands/export.py   |   20 +++++++-------------
 stgit/commands/files.py    |   15 +++++----------
 stgit/commands/goto.py     |    2 +-
 stgit/commands/mail.py     |   26 +++++++++-----------------
 stgit/commands/status.py   |   17 +++++------------
 stgit/git.py               |    9 +++------
 stgit/lib/git.py           |   42 ++++++++++++++++++++++++++++++++----------
 stgit/lib/transaction.py   |    4 ++--
 stgit/utils.py             |   20 ++++++++++++++------
 t/t2500-clean.sh           |   17 +++++++++++++++++
 17 files changed, 119 insertions(+), 108 deletions(-)

                                 -+-

The following changes since commit 149ad73c6b1639981b1064a9e8f3699b08928621:
  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 experimental

Karl Hasselström (6):
      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
      Convert "stg edit" to the new infrastructure

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

 examples/gitconfig        |    4 +
 stgit/commands/edit.py    |  309 +++++++++++++++++++++------------------------
 stgit/commands/refresh.py |   25 +++-
 stgit/commands/show.py    |   13 +--
 stgit/lib/git.py          |   54 ++++++++
 stgit/utils.py            |    3 +-
 t/t2700-refresh.sh        |   57 ++++++++-
 7 files changed, 283 insertions(+), 182 deletions(-)

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

[StGit PATCH 0/5] Various cleanups

[Karl Hasselström]

Some of these are very tiny, others are a bit bigger. They're all in
kha/safe, since they should be perfectly safe.

---

Karl Hasselström (5):
      Let the caller supply the diff text to diffstat()
      Simplify editor selection logic
      Eliminate temp variable that's used just once
      Wrap excessively long line
      Create index and worktree objects just once


 stgit/commands/coalesce.py |    2 +-
 stgit/commands/commit.py   |    2 +-
 stgit/commands/common.py   |    3 ++-
 stgit/commands/diff.py     |    9 ++++-----
 stgit/commands/edit.py     |    2 +-
 stgit/commands/export.py   |   10 +++++-----
 stgit/commands/files.py    |    2 +-
 stgit/commands/goto.py     |    2 +-
 stgit/commands/mail.py     |   16 +++++++---------
 stgit/git.py               |    9 +++------
 stgit/lib/git.py           |   34 ++++++++++++++++++++++++----------
 stgit/lib/transaction.py   |    3 +--
 stgit/utils.py             |    8 ++------
 13 files changed, 53 insertions(+), 49 deletions(-)

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

[StGit PATCH 1/5] Create index and worktree objects just once

[Karl Hasselström]

Create the objects for a repository's default index, worktree, and
index+worktree just once. Both for performance (though the gain is
probably negligible), and for future-proofing if we ever add mutable
state to those objects.

And make them properties while we're at it.

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

---

 stgit/commands/coalesce.py |    2 +-
 stgit/commands/commit.py   |    2 +-
 stgit/commands/goto.py     |    2 +-
 stgit/lib/git.py           |   34 ++++++++++++++++++++++++----------
 4 files changed, 27 insertions(+), 13 deletions(-)


diff --git a/stgit/commands/coalesce.py b/stgit/commands/coalesce.py
index d2cba3e..291a537 100644
--- a/stgit/commands/coalesce.py
+++ b/stgit/commands/coalesce.py
@@ -118,5 +118,5 @@ def func(parser, options, args):
                                           + list(stack.patchorder.unapplied)))
     if len(patches) < 2:
         raise common.CmdException('Need at least two patches')
-    return _coalesce(stack, stack.repository.default_iw(), options.name,
+    return _coalesce(stack, stack.repository.default_iw, options.name,
                      options.message, options.save_template, patches)
diff --git a/stgit/commands/commit.py b/stgit/commands/commit.py
index 1d741b3..bff94ce 100644
--- a/stgit/commands/commit.py
+++ b/stgit/commands/commit.py
@@ -68,7 +68,7 @@ def func(parser, options, args):
     if not patches:
         raise common.CmdException('No patches to commit')
 
-    iw = stack.repository.default_iw()
+    iw = stack.repository.default_iw
     trans = transaction.StackTransaction(stack, 'stg commit')
     try:
         common_prefix = 0
diff --git a/stgit/commands/goto.py b/stgit/commands/goto.py
index 763a8af..fe13e49 100644
--- a/stgit/commands/goto.py
+++ b/stgit/commands/goto.py
@@ -35,7 +35,7 @@ def func(parser, options, args):
     patch = args[0]
 
     stack = directory.repository.current_stack
-    iw = stack.repository.default_iw()
+    iw = stack.repository.default_iw
     trans = transaction.StackTransaction(stack, 'stg goto')
     if patch in trans.applied:
         to_pop = set(trans.applied[trans.applied.index(patch)+1:])
diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 118c9b2..2af1844 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -211,6 +211,9 @@ class Repository(RunWithEnv):
         self.__refs = Refs(self)
         self.__trees = ObjectCache(lambda sha1: Tree(sha1))
         self.__commits = ObjectCache(lambda sha1: Commit(self, sha1))
+        self.__default_index = None
+        self.__default_worktree = None
+        self.__default_iw = None
     env = property(lambda self: { 'GIT_DIR': self.__git_dir })
     @classmethod
     def default(cls):
@@ -220,21 +223,32 @@ class Repository(RunWithEnv):
                                ).output_one_line())
         except run.RunException:
             raise RepositoryException('Cannot find git repository')
+    @property
     def default_index(self):
-        return Index(self, (os.environ.get('GIT_INDEX_FILE', None)
-                            or os.path.join(self.__git_dir, 'index')))
+        if self.__default_index == None:
+            self.__default_index = Index(
+                self, (os.environ.get('GIT_INDEX_FILE', None)
+                       or os.path.join(self.__git_dir, 'index')))
+        return self.__default_index
     def temp_index(self):
         return Index(self, self.__git_dir)
+    @property
     def default_worktree(self):
-        path = os.environ.get('GIT_WORK_TREE', None)
-        if not path:
-            o = run.Run('git', 'rev-parse', '--show-cdup').output_lines()
-            o = o or ['.']
-            assert len(o) == 1
-            path = o[0]
-        return Worktree(path)
+        if self.__default_worktree == None:
+            path = os.environ.get('GIT_WORK_TREE', None)
+            if not path:
+                o = run.Run('git', 'rev-parse', '--show-cdup').output_lines()
+                o = o or ['.']
+                assert len(o) == 1
+                path = o[0]
+            self.__default_worktree = Worktree(path)
+        return self.__default_worktree
+    @property
     def default_iw(self):
-        return IndexAndWorktree(self.default_index(), self.default_worktree())
+        if self.__default_iw == None:
+            self.__default_iw = IndexAndWorktree(self.default_index,
+                                                 self.default_worktree)
+        return self.__default_iw
     directory = property(lambda self: self.__git_dir)
     refs = property(lambda self: self.__refs)
     def cat_object(self, sha1):

[StGit PATCH 2/5] Wrap excessively long line

[Karl Hasselström]

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

---

I try to kill those whenever I can get away with it ...

 stgit/commands/common.py |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)


diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index 7d9df02..7a7cb80 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -274,7 +274,8 @@ def name_email(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
+            raise CmdException('Incorrect "name <email>"/"email (name)"'
+                               ' string: %s' % address)
         return ( str_list[0][1], str_list[0][0] )
 
     return str_list[0]

[StGit PATCH 3/5] Eliminate temp variable that's used just once

[Karl Hasselström]

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

---

 stgit/lib/transaction.py |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)


diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 809eabf..6a2ed81 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -174,7 +174,6 @@ class StackTransaction(object):
         """Attempt to push the named patch. If this results in conflicts,
         halts the transaction. If index+worktree are given, spill any
         conflicts to them."""
-        i = self.unapplied.index(pn)
         cd = self.patches[pn].data
         cd = cd.set_committer(None)
         s = ['', ' (empty)'][cd.is_nochange()]
@@ -203,7 +202,7 @@ class StackTransaction(object):
                 s = ' (conflict)'
         cd = cd.set_tree(tree)
         self.patches[pn] = self.__stack.repository.commit(cd)
-        del self.unapplied[i]
+        del self.unapplied[self.unapplied.index(pn)]
         self.applied.append(pn)
         out.info('Pushed %s%s' % (pn, s))
         if merge_conflict:

[StGit PATCH 4/5] Simplify editor selection logic

[Karl Hasselström]

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

---

There are a few other env variables we might like to look at, like
VISUAL. And isn't there a git-specific variable too?

 stgit/utils.py |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)


diff --git a/stgit/utils.py b/stgit/utils.py
index 00776b0..43366c9 100644
--- a/stgit/utils.py
+++ b/stgit/utils.py
@@ -175,12 +175,8 @@ def call_editor(filename):
 
     # the editor
     editor = config.get('stgit.editor')
-    if editor:
-        pass
-    elif 'EDITOR' in os.environ:
-        editor = os.environ['EDITOR']
-    else:
-        editor = 'vi'
+    if not editor:
+        editor = os.environ.get('EDITOR', 'vi')
     editor += ' %s' % filename
 
     out.start('Invoking the editor: "%s"' % editor)

Re: [StGit PATCH 4/5] Simplify editor selection logic

[Peter Oberndorfer]

Subject: [PATCH] use the same search order for picking a editor as git

Signed-off-by: Peter Oberndorfer <kumbayo84@arcor.de>
---
On Dienstag 29 Januar 2008, Karl Hasselström wrote:
> Signed-off-by: Karl Hasselström <kha@treskal.com>
> 
> ---
> 
> There are a few other env variables we might like to look at, like
> VISUAL. And isn't there a git-specific variable too?
> 
>  stgit/utils.py |    8 ++------
>  1 files changed, 2 insertions(+), 6 deletions(-)
> 
> 
> diff --git a/stgit/utils.py b/stgit/utils.py
> index 00776b0..43366c9 100644
> --- a/stgit/utils.py
> +++ b/stgit/utils.py
> @@ -175,12 +175,8 @@ def call_editor(filename):
>  
>      # the editor
>      editor = config.get('stgit.editor')
> -    if editor:
> -        pass
> -    elif 'EDITOR' in os.environ:
> -        editor = os.environ['EDITOR']
> -    else:
> -        editor = 'vi'
> +    if not editor:
> +        editor = os.environ.get('EDITOR', 'vi')
>      editor += ' %s' % filename
>  
>      out.start('Invoking the editor: "%s"' % editor)

Since i personally dislike having a separate config for the editor in git/stgit
i locally use this patch.
unfortunately it makes the whole editor searching thing more complex :-(
But i am sure it is possible to rewrite the code to something easier
with some more python knowledge :-/
So it is not meant for direct applying, just for discussion...

TODO: update sample gitconfig?
TODO: do the same for pager?

 Documentation/stg-new.txt |    6 ++++++
 stgit/utils.py            |   10 +++++++++-
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/Documentation/stg-new.txt b/Documentation/stg-new.txt
index fbf2f67..a728d8e 100644
--- a/Documentation/stg-new.txt
+++ b/Documentation/stg-new.txt
@@ -31,7 +31,10 @@ the patch, unless the '--message' flag already specified one.  The
 editor.  The editor to use is taken from the first of the following
 sources of information, and defaults to 'vi':
 
+. the 'GIT_EDITOR' environment variable
 . the 'stgit.editor' GIT configuration variable
+. the 'core.editor' GIT configuration variable
+. the 'VISUAL' environment variable
 . the 'EDITOR' environment variable
 
 The message and other GIT commit attributes can be modified later
@@ -101,13 +104,16 @@ ENVIRONMENT VARIABLES
 	GIT_AUTHOR_DATE
 	GIT_COMMITTER_NAME
 	GIT_COMMITTER_EMAIL
+	GIT_EDITOR
 	EDITOR
+	VISUAL
 
 GIT CONFIGURATION VARIABLES
 ---------------------------
 
 	user.name
 	user.email
+	core.editor
 	stgit.editor
 
 StGIT
diff --git a/stgit/utils.py b/stgit/utils.py
index 2ff1d74..6b1d196 100644
--- a/stgit/utils.py
+++ b/stgit/utils.py
@@ -174,9 +174,17 @@ def call_editor(filename):
     """Run the editor on the specified filename."""
 
     # the editor
-    editor = config.get('stgit.editor')
+    editor = None
+    if 'GIT_EDITOR' in os.environ:
+        editor = os.environ['GIT_EDITOR']
+    if not editor:
+        editor = config.get('stgit.editor')
+    if not editor:
+        editor = config.get('core.editor')
     if editor:
         pass
+    elif 'VISUAL' in os.environ:
+        editor = os.environ['VISUAL']
     elif 'EDITOR' in os.environ:
         editor = os.environ['EDITOR']
     else:
-- 
1.5.4.rc3

Re: [StGit PATCH 4/5] Simplify editor selection logic

[Karl Hasselström]

On 2008-01-29 21:09:37 +0100, Peter Oberndorfer wrote:

> Since i personally dislike having a separate config for the editor
> in git/stgit i locally use this patch. unfortunately it makes the
> whole editor searching thing more complex :-( But i am sure it is
> possible to rewrite the code to something easier with some more
> python knowledge :-/ So it is not meant for direct applying, just
> for discussion...

I like the intention. What I'd like to do is to let the code _use_ the
stgit.editor variable, but not advertise it in the docs except for
noting that "this is deprecated and will go away in the future, but
for now it's still effective".

> +. the 'GIT_EDITOR' environment variable
>  . the 'stgit.editor' GIT configuration variable
> +. the 'core.editor' GIT configuration variable
> +. the 'VISUAL' environment variable
>  . the 'EDITOR' environment variable

That's a very nice order. (For completeness, the list should end with
"vi", though. And I'd like that deprecation note for stgit.editor,
unless someone has strong objections.)

>      # the editor
> -    editor = config.get('stgit.editor')
> +    editor = None
> +    if 'GIT_EDITOR' in os.environ:
> +        editor = os.environ['GIT_EDITOR']
> +    if not editor:
> +        editor = config.get('stgit.editor')
> +    if not editor:
> +        editor = config.get('core.editor')
>      if editor:
>          pass
> +    elif 'VISUAL' in os.environ:
> +        editor = os.environ['VISUAL']
>      elif 'EDITOR' in os.environ:
>          editor = os.environ['EDITOR']
>      else:

You could write it kind of like this:

  def e(key): return os.environ.get(key, None)
  def c(key): return config.get(key)
  editor = filter(None, [e('GIT_EDITOR'), c('stgit.editor'), c('core.editor'),
                         e('VISUAL'), e('EDITOR'), 'vi'])[0]

Of course, if we're going to have code like this in several places
(you already mentioned the pager), we could build a function like
this:

  editor = get_config(['GIT_EDITOR', 'stgit.editor', 'core.editor',
                       'VISUAL', 'EDITOR'], default = 'vi')

that would differentiate between env variables and conf keys by
looking for dots in the name or something.

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

Re: [StGit PATCH 4/5] Simplify editor selection logic

[Jay Soffian]

On Jan 30, 2008 2:28 AM, Karl Hasselström <kha@treskal.com> wrote:
> You could write it kind of like this:
>
>   def e(key): return os.environ.get(key, None)
>   def c(key): return config.get(key)
>   editor = filter(None, [e('GIT_EDITOR'), c('stgit.editor'), c('core.editor'),
>                          e('VISUAL'), e('EDITOR'), 'vi'])[0]

Too clever by half if you ask me. Why not just:

editor = (os.environ.get('GIT_EDITOR') or
          config.get('stgit.editor') or
          config.get('core.editor') or
          os.environ.get('VISUAL') or
          os.environ.get('EDITOR') or
          'vi')

And be done with it?

> Of course, if we're going to have code like this in several places
> (you already mentioned the pager), we could build a function like
> this:
>
>   editor = get_config(['GIT_EDITOR', 'stgit.editor', 'core.editor',
>                        'VISUAL', 'EDITOR'], default = 'vi')
>
> that would differentiate between env variables and conf keys by
> looking for dots in the name or something.

def get_config(keys, default=None):
    rv = default
    for k in keys:
        if '.' in k:
            d = config
        else:
            d = os.environ
        if k in d:
            rv = d[k]
            break
    return rv

j.

Re: [StGit PATCH 4/5] Simplify editor selection logic

[Karl Hasselström]

On 2008-01-30 09:55:18 -0500, Jay Soffian wrote:

> On Jan 30, 2008 2:28 AM, Karl Hasselström <kha@treskal.com> wrote:

> > You could write it kind of like this:
> >
> >   def e(key): return os.environ.get(key, None)
> >   def c(key): return config.get(key)
> >   editor = filter(None, [e('GIT_EDITOR'), c('stgit.editor'), c('core.editor'),
> >                          e('VISUAL'), e('EDITOR'), 'vi'])[0]
>
> Too clever by half if you ask me. Why not just:
>
> editor = (os.environ.get('GIT_EDITOR') or
>           config.get('stgit.editor') or
>           config.get('core.editor') or
>           os.environ.get('VISUAL') or
>           os.environ.get('EDITOR') or
>           'vi')
>
> And be done with it?

Yes. It's more repetitive, but not much longer. With only five options
and one default -- if there were more, my version would be nicer
(IMHO).

> > Of course, if we're going to have code like this in several places
> > (you already mentioned the pager), we could build a function like
> > this:
> >
> >   editor = get_config(['GIT_EDITOR', 'stgit.editor', 'core.editor',
> >                        'VISUAL', 'EDITOR'], default = 'vi')
> >
> > that would differentiate between env variables and conf keys by
> > looking for dots in the name or something.
> 
> def get_config(keys, default=None):
>     rv = default
>     for k in keys:
>         if '.' in k:
>             d = config
>         else:
>             d = os.environ
>         if k in d:
>             rv = d[k]
>             break
>     return rv

'config' isn't a dict, so you have to use config.get, and you can't
use 'k in config'. But otherwise yes. So something like this maybe:

def get_config(keys, default = None):
    rv = None
    for k in keys:
        if '.' in k:
            rv = config.get(k)
        else:
            rv = os.environ.get(k, None)
        if rv != None:
            return rv
    return default

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

[StGit PATCH 5/5] Let the caller supply the diff text to diffstat()

[Karl Hasselström]

Almost all diffstat() callers already have the diff text, so they
might as well pass it to diffstat() instead of letting it recompute
it. In some cases this even makes for a code simplification since the
diff (and thus diffstat) parameters were nontrivial.

Also, diffstat() wasn't as versatile as diff(); for example, it didn't
accept any extra diff options. This patch solves all of those problems
as a side-effect.

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

---

I wanted this for the "stg edit" rewrite. It took some time before I
saw the TODO comment describing exactly the change I was in the
process of making!

 stgit/commands/diff.py   |    9 ++++-----
 stgit/commands/edit.py   |    2 +-
 stgit/commands/export.py |   10 +++++-----
 stgit/commands/files.py  |    2 +-
 stgit/commands/mail.py   |   16 +++++++---------
 stgit/git.py             |    9 +++------
 6 files changed, 21 insertions(+), 27 deletions(-)


diff --git a/stgit/commands/diff.py b/stgit/commands/diff.py
index 7c213d1..fd6be34 100644
--- a/stgit/commands/diff.py
+++ b/stgit/commands/diff.py
@@ -81,12 +81,11 @@ def func(parser, options, args):
         rev1 = 'HEAD'
         rev2 = None
 
+    diff_str = git.diff(args, git_id(crt_series, rev1),
+                        git_id(crt_series, rev2),
+                        diff_flags = options.diff_flags)
     if options.stat:
-        out.stdout_raw(git.diffstat(args, git_id(crt_series, rev1),
-                                    git_id(crt_series, rev2)) + '\n')
+        out.stdout_raw(git.diffstat(diff_str) + '\n')
     else:
-        diff_str = git.diff(args, git_id(crt_series, rev1),
-                            git_id(crt_series, rev2),
-                            diff_flags = options.diff_flags)
         if diff_str:
             pager(diff_str)
diff --git a/stgit/commands/edit.py b/stgit/commands/edit.py
index da67275..9915e49 100644
--- a/stgit/commands/edit.py
+++ b/stgit/commands/edit.py
@@ -154,9 +154,9 @@ def __generate_file(pname, write_fn, options):
                 '%(diffstat)s\n' \
                 '%(diff)s'
 
-        tmpl_dict['diffstat'] = git.diffstat(rev1 = bottom, rev2 = top)
         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
diff --git a/stgit/commands/export.py b/stgit/commands/export.py
index 16c64ba..50f6f67 100644
--- a/stgit/commands/export.py
+++ b/stgit/commands/export.py
@@ -138,11 +138,13 @@ def func(parser, options, args):
         long_descr = reduce(lambda x, y: x + '\n' + y,
                             descr_lines[1:], '').strip()
 
+        diff = git.diff(rev1 = patch.get_bottom(),
+                        rev2 = patch.get_top(),
+                        diff_flags = options.diff_flags)
         tmpl_dict = {'description': patch.get_description().rstrip(),
                      'shortdescr': short_descr,
                      'longdescr': long_descr,
-                     'diffstat': git.diffstat(rev1 = patch.get_bottom(),
-                                              rev2 = patch.get_top()),
+                     'diffstat': git.diffstat(diff),
                      'authname': patch.get_authname(),
                      'authemail': patch.get_authemail(),
                      'authdate': patch.get_authdate(),
@@ -172,9 +174,7 @@ def func(parser, options, args):
             print '-'*79
 
         f.write(descr)
-        f.write(git.diff(rev1 = patch.get_bottom(),
-                         rev2 = patch.get_top(),
-                         diff_flags = options.diff_flags))
+        f.write(diff)
         if not options.stdout:
             f.close()
         patch_no += 1
diff --git a/stgit/commands/files.py b/stgit/commands/files.py
index ab1f6a3..b43b12f 100644
--- a/stgit/commands/files.py
+++ b/stgit/commands/files.py
@@ -60,7 +60,7 @@ def func(parser, options, args):
     rev2 = git_id(crt_series, '%s//top' % patch)
 
     if options.stat:
-        out.stdout_raw(git.diffstat(rev1 = rev1, rev2 = rev2) + '\n')
+        out.stdout_raw(git.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/mail.py b/stgit/commands/mail.py
index 4aa16fb..7d19eca 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -360,9 +360,9 @@ 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(
+                 'diffstat':     git.diffstat(git.diff(
                      rev1 = git_id(crt_series, '%s//bottom' % patches[0]),
-                     rev2 = git_id(crt_series, '%s//top' % patches[-1]))}
+                     rev2 = git_id(crt_series, '%s//top' % patches[-1])))}
 
     try:
         msg_string = tmpl % tmpl_dict
@@ -433,6 +433,9 @@ def __build_message(tmpl, patch, patch_nr, total_nr, msg_id, ref_id, options):
     else:
         number_str = ''
 
+    diff = git.diff(rev1 = git_id(crt_series, '%s//bottom' % patch),
+                    rev2 = git_id(crt_series, '%s//top' % patch),
+                    diff_flags = options.diff_flags)
     tmpl_dict = {'patch':        patch,
                  'sender':       sender,
                  # for backward template compatibility
@@ -441,13 +444,8 @@ def __build_message(tmpl, patch, patch_nr, total_nr, msg_id, ref_id, options):
                  'longdescr':    long_descr,
                  # for backward template compatibility
                  'endofheaders': '',
-                 'diff':         git.diff(
-                     rev1 = git_id(crt_series, '%s//bottom' % patch),
-                     rev2 = git_id(crt_series, '%s//top' % patch),
-                     diff_flags = options.diff_flags),
-                 'diffstat':     git.diffstat(
-                     rev1 = git_id(crt_series, '%s//bottom'%patch),
-                     rev2 = git_id(crt_series, '%s//top' % patch)),
+                 'diff':         diff,
+                 'diffstat':     git.diffstat(diff),
                  # for backward template compatibility
                  'date':         '',
                  'version':      version_str,
diff --git a/stgit/git.py b/stgit/git.py
index 85cceb0..4dc4dcf 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -640,12 +640,9 @@ def diff(files = None, rev1 = 'HEAD', rev2 = None, diff_flags = [],
     else:
         return ''
 
-# TODO: take another parameter representing a diff string as we
-# usually invoke git.diff() form the calling functions
-def diffstat(files = None, rev1 = 'HEAD', rev2 = None):
-    """Return the diffstat between rev1 and rev2."""
-    return GRun('apply', '--stat', '--summary'
-                ).raw_input(diff(files, rev1, rev2)).raw_output()
+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

[StGit PATCH 0/2] "stg clean" test+bugfix

[Karl Hasselström]

This is the test by Pavel, followed by a fix by me.

They're in kha/safe, since they should be low-risk, and because losing
a conflicting patch (even though you still have all the changes in
your worktree) is unpleasant.

---

Karl Hasselström (1):
      Don't clean away patches with conflicts

Pavel Roskin (1):
      Add test to ensure that "stg clean" preserves conflicting patches


 stgit/commands/clean.py |    7 +++++++
 stgit/lib/git.py        |    8 ++++++++
 t/t2500-clean.sh        |   17 +++++++++++++++++
 3 files changed, 32 insertions(+), 0 deletions(-)

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

[StGit PATCH 1/2] Add test to ensure that "stg clean" preserves conflicting patches

[Karl Hasselström]

From: Pavel Roskin <proski@gnu.org>

If "stg push" fails, the subsequent "stg clean" will remove the patch
that could not be applied. I think it's wrong. Especially when doing
"stg pull", it can happen that I want to run "stg clean" to get rid of
the patches applied upstream so I can concentrate on the conflict.
Instead, the conflicting patch is removed too.

The test added by this patch should pass once the bug is fixed.

Signed-off-by: Pavel Roskin <proski@gnu.org>
Signed-off-by: Karl Hasselström <kha@treskal.com>

---

I doctored the commit message a bit, and made the failing test use
test_expect_failure.

 t/t2500-clean.sh |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)


diff --git a/t/t2500-clean.sh b/t/t2500-clean.sh
index 3364c18..b38d868 100755
--- a/t/t2500-clean.sh
+++ b/t/t2500-clean.sh
@@ -24,4 +24,21 @@ test_expect_success 'Clean empty patches' '
     [ "$(echo $(stg unapplied))" = "" ]
 '
 
+test_expect_success 'Create a conflict' '
+    stg new p1 -m p1 &&
+    echo bar > foo.txt &&
+    stg refresh &&
+    stg pop &&
+    stg new p2 -m p2
+    echo quux > foo.txt &&
+    stg refresh &&
+    ! stg push
+'
+
+test_expect_failure 'Make sure conflicting patches are preserved' '
+    stg clean &&
+    [ "$(echo $(stg applied))" = "p0 p2 p1" ] &&
+    [ "$(echo $(stg unapplied))" = "" ]
+'
+
 test_done

[StGit PATCH 2/2] Don't clean away patches with conflicts

[Karl Hasselström]

If we have conflicts, it means that the topmost patch is empty because
of those conflicts (since StGit explicitly makes a conflicting patch
empty), so don't let "stg clean" touch it.

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

---

I considered fixing this by way of a check in the shared
new-infrastructure code, but came to the conclusion that the check
really does belong in "stg clean".

 stgit/commands/clean.py |    7 +++++++
 stgit/lib/git.py        |    8 ++++++++
 t/t2500-clean.sh        |    2 +-
 3 files changed, 16 insertions(+), 1 deletions(-)


diff --git a/stgit/commands/clean.py b/stgit/commands/clean.py
index a0a0dca..889c1dc 100644
--- a/stgit/commands/clean.py
+++ b/stgit/commands/clean.py
@@ -40,6 +40,13 @@ def _clean(stack, clean_applied, clean_unapplied):
     trans = transaction.StackTransaction(stack, 'stg clean')
     def del_patch(pn):
         if pn in stack.patchorder.applied:
+            if pn == stack.patchorder.applied[-1]:
+                # We're about to clean away the topmost patch. Don't
+                # do that if we have conflicts, since that means the
+                # patch is only empty because the conflicts have made
+                # us dump its contents into the index and worktree.
+                if stack.repository.default_index.conflicts():
+                    return False
             return clean_applied and trans.patches[pn].data.is_nochange()
         elif pn in stack.patchorder.unapplied:
             return clean_unapplied and trans.patches[pn].data.is_nochange()
diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 2af1844..6cd7450 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -353,6 +353,14 @@ class Index(RunWithEnv):
     def delete(self):
         if os.path.isfile(self.__filename):
             os.remove(self.__filename)
+    def conflicts(self):
+        """The set of conflicting paths."""
+        paths = set()
+        for line in self.run(['git', 'ls-files', '-z', '--unmerged']
+                             ).raw_output().split('\0')[:-1]:
+            stat, path = line.split('\t', 1)
+            paths.add(path)
+        return paths
 
 class Worktree(object):
     def __init__(self, directory):
diff --git a/t/t2500-clean.sh b/t/t2500-clean.sh
index b38d868..ad8f892 100755
--- a/t/t2500-clean.sh
+++ b/t/t2500-clean.sh
@@ -35,7 +35,7 @@ test_expect_success 'Create a conflict' '
     ! stg push
 '
 
-test_expect_failure 'Make sure conflicting patches are preserved' '
+test_expect_success 'Make sure conflicting patches are preserved' '
     stg clean &&
     [ "$(echo $(stg applied))" = "p0 p2 p1" ] &&
     [ "$(echo $(stg unapplied))" = "" ]

[StGit PATCH 0/4] Rewrite "stg edit" to use new infrastructure

[Karl Hasselström]

This is in kha/experimental. We hardly have any tests for "stg edit",
and the control flow is kind of complicated.

---

Karl Hasselström (4):
      Convert "stg edit" to the new infrastructure
      Teach new infrastructure to diff two trees
      Teach new infrastructure to apply patches
      Teach new infrastructure about the default author and committer


 stgit/commands/edit.py |  309 ++++++++++++++++++++++--------------------------
 stgit/lib/git.py       |   54 ++++++++
 2 files changed, 196 insertions(+), 167 deletions(-)

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

[StGit PATCH 1/4] Teach new infrastructure about the default author and committer

[Karl Hasselström]

As specified by the config options user.name and user.email, and the
environment variables GIT_{AUTHOR,COMMITTER}_{NAME,EMAIL,DATE} (the
latter overriding the former).

Nothing uses this yet, but "stg edit" will soon.

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

---

 stgit/lib/git.py |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)


diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 6cd7450..8678979 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -52,6 +52,30 @@ class Person(Repr):
         email = m.group(2)
         date = m.group(3)
         return cls(name, email, date)
+    @classmethod
+    def user(cls):
+        if not hasattr(cls, '__user'):
+            cls.__user = cls(name = config.get('user.name'),
+                             email = config.get('user.email'))
+        return cls.__user
+    @classmethod
+    def author(cls):
+        if not hasattr(cls, '__author'):
+            cls.__author = cls(
+                name = os.environ.get('GIT_AUTHOR_NAME', NoValue),
+                email = os.environ.get('GIT_AUTHOR_EMAIL', NoValue),
+                date = os.environ.get('GIT_AUTHOR_DATE', NoValue),
+                defaults = cls.user())
+        return cls.__author
+    @classmethod
+    def committer(cls):
+        if not hasattr(cls, '__committer'):
+            cls.__committer = cls(
+                name = os.environ.get('GIT_COMMITTER_NAME', NoValue),
+                email = os.environ.get('GIT_COMMITTER_EMAIL', NoValue),
+                date = os.environ.get('GIT_COMMITTER_DATE', NoValue),
+                defaults = cls.user())
+        return cls.__committer
 
 class Tree(Repr):
     """Immutable."""

[StGit PATCH 2/4] Teach new infrastructure to apply patches

[Karl Hasselström]

Two new methods: one index method that applies a patch to that index
or fails without side-effects (without touching a worktree in either
case); and one repository method that uses a temp index to apply a
patch to a tree and returning the new tree (or None if the application
failed), entirely side-effect free.

Nothing uses this yet, but "stg edit" will soon.

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

---

 stgit/lib/git.py |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)


diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 8678979..9cb2521 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -338,6 +338,23 @@ class Repository(RunWithEnv):
                 return None
         finally:
             index.delete()
+    def apply(self, tree, patch_text):
+        """Given a tree and a patch, will either return the new tree that
+        results when the patch is applied, or None if the patch
+        couldn't be applied."""
+        assert isinstance(tree, Tree)
+        if not patch_text:
+            return tree
+        index = self.temp_index()
+        try:
+            index.read_tree(tree)
+            try:
+                index.apply(patch_text)
+                return index.write_tree()
+            except MergeException:
+                return None
+        finally:
+            index.delete()
 
 class MergeException(exception.StgException):
     pass
@@ -374,6 +391,13 @@ class Index(RunWithEnv):
         """In-index merge, no worktree involved."""
         self.run(['git', 'read-tree', '-m', '-i', '--aggressive',
                   base.sha1, ours.sha1, theirs.sha1]).no_output()
+    def apply(self, patch_text):
+        """In-index patch application, no worktree involved."""
+        try:
+            self.run(['git', 'apply', '--cached']
+                     ).raw_input(patch_text).no_output()
+        except run.RunException:
+            raise MergeException('Patch does not apply cleanly')
     def delete(self):
         if os.path.isfile(self.__filename):
             os.remove(self.__filename)

[StGit PATCH 3/4] Teach new infrastructure to diff two trees

[Karl Hasselström]

Nothing uses this yet, but "stg edit" will soon.

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

---

 stgit/lib/git.py |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)


diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 9cb2521..d75f724 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -1,5 +1,6 @@
 import os, os.path, re
 from stgit import exception, run, utils
+from stgit.config import config
 
 class RepositoryException(exception.StgException):
     pass
@@ -355,6 +356,11 @@ class Repository(RunWithEnv):
                 return None
         finally:
             index.delete()
+    def diff_tree(self, t1, t2, diff_opts):
+        assert isinstance(t1, Tree)
+        assert isinstance(t2, Tree)
+        return self.run(['git', 'diff-tree', '-p'] + list(diff_opts)
+                        + [t1.sha1, t2.sha1]).raw_output()
 
 class MergeException(exception.StgException):
     pass

Re: [StGit PATCH 3/4] Teach new infrastructure to diff two trees

[David Kågedal]

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

> Nothing uses this yet, but "stg edit" will soon.
>
> Signed-off-by: Karl Hasselström <kha@treskal.com>
>
> ---
>
>  stgit/lib/git.py |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
>
>
> diff --git a/stgit/lib/git.py b/stgit/lib/git.py
> index 9cb2521..d75f724 100644
> --- a/stgit/lib/git.py
> +++ b/stgit/lib/git.py
> @@ -1,5 +1,6 @@
>  import os, os.path, re
>  from stgit import exception, run, utils
> +from stgit.config import config

But "config" isn't used in this patch.

>  class RepositoryException(exception.StgException):
>      pass
> @@ -355,6 +356,11 @@ class Repository(RunWithEnv):
>                  return None
>          finally:
>              index.delete()
> +    def diff_tree(self, t1, t2, diff_opts):
> +        assert isinstance(t1, Tree)
> +        assert isinstance(t2, Tree)
> +        return self.run(['git', 'diff-tree', '-p'] + list(diff_opts)
> +                        + [t1.sha1, t2.sha1]).raw_output()
>  
>  class MergeException(exception.StgException):
>      pass
>

-- 
David Kågedal

Re: [StGit PATCH 3/4] Teach new infrastructure to diff two trees

[Karl Hasselström]

On 2008-01-29 15:40:55 +0100, David Kågedal wrote:

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

> > +from stgit.config import config
>
> But "config" isn't used in this patch.

You're right, it's not. It's actually used in patch 1/4, but because
that code is never called until patch 4/4, the interpreter didn't
catch the error.

Thanks, will fix.

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

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

[Karl Hasselström]

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>

---

Testers welcome!

 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..b42728f 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,
+            '',
+            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(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 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 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', 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()

[StGit PATCH 0/3] "stg edit" fixups

[Karl Hasselström]

I've fixed a few things with the "stg edit" series:

  * Took care of the import problem David pointed out.

  * Made the emacs mode not refuse to edit unaplied patches, now that
    "stg edit" can do it.

  * Made it so that the dates the user gets to edit are nicely
    human-readable, and not that seconds-since-1970 mess.

Also pushed out to kha/experimental.

---

Karl Hasselström (3):
      It's possible to edit unapplied patches now
      Convert "stg edit" to the new infrastructure
      Parse the date instead of treating it as an opaque string


 contrib/stgit.el       |    4 -
 stgit/commands/edit.py |  309 ++++++++++++++++++++++--------------------------
 stgit/lib/git.py       |   76 +++++++++++-
 3 files changed, 215 insertions(+), 174 deletions(-)

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

[StGit PATCH 1/3] Parse the date instead of treating it as an opaque string

[Karl Hasselström]

This is needed for when we want to display it, since the
seconds-since-the-epoch format isn't that human-friendly.

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

---

 stgit/lib/git.py |   76 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 72 insertions(+), 4 deletions(-)


diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index d75f724..50dc4f1 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -1,10 +1,17 @@
 import os, os.path, re
+from datetime import datetime, timedelta, tzinfo
+
 from stgit import exception, run, utils
 from stgit.config import config
 
 class RepositoryException(exception.StgException):
     pass
 
+class DateException(exception.StgException):
+    def __init__(self, string, type):
+        exception.StgException.__init__(
+            self, '"%s" is not a valid %s' % (string, type))
+
 class DetachedHeadException(RepositoryException):
     def __init__(self):
         RepositoryException.__init__(self, 'Not on any branch')
@@ -26,6 +33,65 @@ def make_defaults(defaults):
             return None
     return d
 
+class TimeZone(tzinfo, Repr):
+    def __init__(self, tzstring):
+        m = re.match(r'^([+-])(\d{2}):?(\d{2})$', tzstring)
+        if not m:
+            raise DateException(tzstring, 'time zone')
+        sign = int(m.group(1) + '1')
+        try:
+            self.__offset = timedelta(hours = sign*int(m.group(2)),
+                                      minutes = sign*int(m.group(3)))
+        except OverflowError:
+            raise DateException(tzstring, 'time zone')
+        self.__name = tzstring
+    def utcoffset(self, dt):
+        return self.__offset
+    def tzname(self, dt):
+        return self.__name
+    def dst(self, dt):
+        return timedelta(0)
+    def __str__(self):
+        return self.__name
+
+class Date(Repr):
+    """Immutable."""
+    def __init__(self, datestring):
+        # Try git-formatted date.
+        m = re.match(r'^(\d+)\s+([+-]\d\d:?\d\d)$', datestring)
+        if m:
+            try:
+                self.__time = datetime.fromtimestamp(int(m.group(1)),
+                                                     TimeZone(m.group(2)))
+            except ValueError:
+                raise DateException(datestring, 'date')
+            return
+
+        # Try iso-formatted date.
+        m = re.match(r'^(\d{4})-(\d{2})-(\d{2})\s+(\d{2}):(\d{2}):(\d{2})\s+'
+                     + r'([+-]\d\d:?\d\d)$', datestring)
+        if m:
+            try:
+                self.__time = datetime(
+                    *[int(m.group(i + 1)) for i in xrange(6)],
+                    **{'tzinfo': TimeZone(m.group(7))})
+            except ValueError:
+                raise DateException(datestring, 'date')
+            return
+
+        raise DateException(datestring, 'date')
+    def __str__(self):
+        return self.isoformat()
+    def isoformat(self):
+        """Human-friendly ISO 8601 format."""
+        return '%s %s' % (self.__time.replace(tzinfo = None).isoformat(' '),
+                          self.__time.tzinfo)
+    @classmethod
+    def maybe(cls, datestring):
+        if datestring in [None, NoValue]:
+            return datestring
+        return cls(datestring)
+
 class Person(Repr):
     """Immutable."""
     def __init__(self, name = NoValue, email = NoValue,
@@ -34,6 +100,7 @@ class Person(Repr):
         self.__name = d(name, 'name')
         self.__email = d(email, 'email')
         self.__date = d(date, 'date')
+        assert isinstance(self.__date, Date) or self.__date in [None, NoValue]
     name = property(lambda self: self.__name)
     email = property(lambda self: self.__email)
     date = property(lambda self: self.__date)
@@ -51,7 +118,7 @@ class Person(Repr):
         assert m
         name = m.group(1).strip()
         email = m.group(2)
-        date = m.group(3)
+        date = Date(m.group(3))
         return cls(name, email, date)
     @classmethod
     def user(cls):
@@ -65,7 +132,7 @@ class Person(Repr):
             cls.__author = cls(
                 name = os.environ.get('GIT_AUTHOR_NAME', NoValue),
                 email = os.environ.get('GIT_AUTHOR_EMAIL', NoValue),
-                date = os.environ.get('GIT_AUTHOR_DATE', NoValue),
+                date = Date.maybe(os.environ.get('GIT_AUTHOR_DATE', NoValue)),
                 defaults = cls.user())
         return cls.__author
     @classmethod
@@ -74,7 +141,8 @@ class Person(Repr):
             cls.__committer = cls(
                 name = os.environ.get('GIT_COMMITTER_NAME', NoValue),
                 email = os.environ.get('GIT_COMMITTER_EMAIL', NoValue),
-                date = os.environ.get('GIT_COMMITTER_DATE', NoValue),
+                date = Date.maybe(
+                    os.environ.get('GIT_COMMITTER_DATE', NoValue)),
                 defaults = cls.user())
         return cls.__committer
 
@@ -301,7 +369,7 @@ class Repository(RunWithEnv):
                 for attr, v2 in (('name', 'NAME'), ('email', 'EMAIL'),
                                  ('date', 'DATE')):
                     if getattr(p, attr) != None:
-                        env['GIT_%s_%s' % (v1, v2)] = getattr(p, attr)
+                        env['GIT_%s_%s' % (v1, v2)] = str(getattr(p, attr))
         sha1 = self.run(c, env = env).raw_input(commitdata.message
                                                 ).output_one_line()
         return self.get_commit(sha1)

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

[Karl Hasselström]

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()

[StGit PATCH 3/3] It's possible to edit unapplied patches now

[Karl Hasselström]

With the rewrite, "stg edit" gained the ability to edit unapplied
patches, so the emacs mode no longer has to check that a patch is
applied before trying to edit it.

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

---

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


diff --git a/contrib/stgit.el b/contrib/stgit.el
index e8bbb2c..bef41c7 100644
--- a/contrib/stgit.el
+++ b/contrib/stgit.el
@@ -290,9 +290,7 @@ Commands:
 (defun stgit-edit ()
   "Edit the patch on the current line"
   (interactive)
-  (let ((patch (if (stgit-applied-at-point)
-                   (stgit-patch-at-point)
-                 (error "This patch is not applied")))
+  (let ((patch (stgit-patch-at-point))
         (edit-buf (get-buffer-create "*StGit edit*"))
         (dir default-directory))
     (log-edit 'stgit-confirm-edit t nil edit-buf)