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@xxxxxxxxxxx
www.treskal.com/kalle
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
