Thanks, everybody, for all of the feedback. This is the redone patch
series, which I think addresses all of your suggestions.
I would still like to implement "don't open the editor if there are
only fixups", but if it's OK I'll work on that in a separate patch
series when I have time.
Michael J Gruber wrote:
> My first reaction to the subject was "Huh? What repository?". So I
> suggest something like
>
> t3404: Better document the original repository layout
>
> as a more descriptive subject.
Good idea. Done.
Johannes Schindelin wrote:
> Alternatively, one could use the test_commit function, I guess. [...]
Yes, that's much easier. Changed. This makes the old first and
second patches reduce to a single one.
Johannes Schindelin wrote:
> On Fri, 4 Dec 2009, Junio C Hamano wrote:
>> Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:
>>
>>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>>> index 0bd3bf7..539413d 100755
>>> --- a/git-rebase--interactive.sh
>>> +++ b/git-rebase--interactive.sh
>>> @@ -302,7 +302,7 @@ nth_string () {
>>>
>>> make_squash_message () {
>>> if test -f "$SQUASH_MSG"; then
>>> - COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
>>> + COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)\(th\|st\|nd\|rd\) commit message.*:/\1/p" \
>>> < "$SQUASH_MSG" | sed -ne '$p')+1))
>> This sed replacement worries me. I don't have a time to check myself
>> today but do we use \(this\|or\|that\) alternates with our sed script
>> already elsewhere in the codebase (test scripts do not count)?
>>
>> Otherwise this may suddenly be breaking a platform that has an
>> implementation of sed that may be substandard but so far has been
>> sufficient to work with git.
>
> IIRC "|" was not correctly handled by BSD sed (used e.g. in MacOSX).
>
> So maybe it would be best to just look for "commit message"? I agree with
> Michael that the regex should not be too loose.
Thanks for pointing this out. I replaced the problematic part of the
regexp with "[snrt][tdh]", which isn't quite so selective but should
be portable. (This is the same fix that Junio suggested.)
Björn Gustavsson wrote:
> On Fri, Dec 4, 2009 at 3:36 PM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
> Nitpick: the recommended style is to leave out the full stop
> at the end of the first line of the commit message.
Nit picked.
Junio C Hamano wrote:
> IIRC, the end result of the bikeshedding for the name of the command was
> won by Dscho's "fixup":
>
> http://thread.gmane.org/gmane.comp.version-control.git/127923/focus=121820
That's fine with me (the abbreviation is even the same :-) ).
Changed.
Michael
Michael Haggerty (2):
t3404: Use test_commit to set up test repository
Add a command "fixup" to rebase --interactive
Documentation/git-rebase.txt | 13 +++--
git-rebase--interactive.sh | 51 +++++++++++++++++----
t/lib-rebase.sh | 7 ++-
t/t3404-rebase-interactive.sh | 96 +++++++++++++++++++++-------------------
4 files changed, 103 insertions(+), 64 deletions(-)
--
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