Subject: [PATCH] fix stg edit command

From: Peter Oberndorfer <kumbayo84@arcor.de>
Date: Feb 12, 2008, 22:05 UTC
Message-ID: <200802122305.05696.kumbayo84@arcor.de>
In-Reply-To: <20080210204359.17683.41935.stgit@yoghurt>

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:

Show 5 quoted lines

> 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.

Show 14 quoted lines

> -    # 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