[PATCHv2 0/2] Add a "fixup" command to "rebase --interactive"
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Dec 7, 2009, 04:22 UTC
- Message-ID
- <cover.1260099005.git.mhagger@alum.mit.edu>
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:
Show 6 quoted lines
> 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:
Show 26 quoted lines
> On Fri, 4 Dec 2009, Junio C Hamano wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> 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@alum.mit.edu> 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(-)