Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[Michael Haggerty <mhagger@alum.mit.edu>]

On 03/02/2014 10:09 AM, Eric Sunshine wrote:
> On Sun, Mar 2, 2014 at 4:04 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>>> "git rebase -e XYZ" is basically the same as
>>>
>>> EDITOR="sed -i '1s/pick XYZ/edit XYZ/' $@" \
>>> git rebase -i XYZ^
>>>
>>> In English, it prepares the todo list for you to edit only commit XYZ
>>> to save your time. The time saving is only significant when you edit a
>>> lot of commits separately.
>>
>> Should this accept multiple -e arguments? Based upon the above
>> justification, it sounds like it should, and I think that would be the
>> intuitive expectation (at least for me).
>>
>> The current implementation, however, is broken with multiple -e arguments. With:
>>
>>     git rebase -i -e older -e newer
>>
>> 'newer' is ignored entirely. However, with:
>>
>>     git rebase -i -e newer -e older
>>
>> it errors out when rewriting the todo list:
>>
>>     "expected to find 'edit older' line but did not"
>>
>> An implementation supporting multiple -e arguments would need to
>> ensure that the todo list extends to the "oldest" rev specified by any
>> -e argument.
> 
> Of course, I'm misreading and abusing the intention of -e as if it is
> "-e <arg>".

I think that your misreading is more consistent than the feature as
implemented.

    git rebase -e OLDER

does not mean "do 'git rebase -i OLDER' and oh, by the way, also set up
commit OLDER to be edited".  It means "do 'git rebase -i OLDER^' ..."
(note: "OLDER^" and not "OLDER").  So it is confusing to think as "-e"
as a modifier on an otherwise normal "git rebase -i" invocation.
Rather, it seems to me that "-e" and "-i" should be mutually exclusive
(and consider it an implementation detail that the former is implemented
using the latter).

And if that is our point of view, then is perfectly logical to allow it
to be specified multiple times.  OTOH there is no reason that v1 has to
allow multiple "-e", as long as it properly rejects that usage.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/