Re: possible usability issue in rebase -i?

[Baz <brian.ewins@gmail.com>]

2009/10/28 Erik Faye-Lund <kusmabite@googlemail.com>:
> On Tue, Oct 27, 2009 at 10:05 PM, Baz <brian.ewins@gmail.com> wrote:
>> I'm fine with this way of fixing it, but I'd make a few more changes...
>
> Feel free to make a patch-series that addresses more issues - I'm not going to.
>

Yep, I wrote one but had to leave the house before sending it. Later today.

> We make patches of one change at the time in Git. Other (related)
> usability issues becomes separate patches, preferably grouped together
> in a patch-series. This change would be one patch in such a series.
>
>>>  OPTIONS_SPEC="\
>>>  git-rebase [-i] [options] [--] <upstream> [<branch>]
>>
>> Use the dashless form and be more consistent with the help - and
>> mention '--root' here, it appears in the
>> help below:
>>
>> -git-rebase [-i] [options] [--] <upstream> [<branch>]
>> +git rebase [--interactive | -i] [options] [--onto <newbase>] [--]
>> <upstream> [<branch>]
>> +git rebase [--interactive | -i] [options] --onto <newbase> --root
>> [--] [<branch>]
>>
>
> I'm not sure I follow - aren't dashless options, uhm, dashless? Do you
> mean to use the long-form instead of the short-form? I'll assume
> that's what you mean for now, since you changed "-i" to "--interactive
> | -i".

No, I just meant 'git rebase' not 'git-rebase'. Sorry, I changed a
couple of things at once.

>
> If so, I'm not 100% convinced it's a clear win: some grep'ing
> indicates that both the short and long form are both widely used, with
> short-option bein a slight favor:
> $ git grep " \[--" | grep -v " \[--\]" | wc -l
>    228
> $ git grep " \[-[^-]" | wc -l
>    243
>
> Also, the usage isn't the only documentation. I think it makes sense
> to try to keep the usage short and to the point, there's a list
> describing each option (showing the full-name) further down in the
> usage-message. And if that's not enough, there's the "git
> help"-command.
>
> If I've misunderstood you and you only want the usage-string to match
> that of the manpage, perhaps that might be a good idea. I dunno.

In the patch I've followed other uses of OPTIONS_SPEC; they're quite
verbose, covering all options, while scripts using USAGE/LONG_USAGE
tend to emit one-liners. As for calling out 'interactive', at the
other extreme its not clear to me why we mention '-i' separately from
'[options]' at all. rebase is already pretty inconsistent here, giving
short or long usage messages depending on whether you passed '-i'. But
I'll take comments on this when I submit the patch, I've no strong
feelings on it.

>
>>
>>> -git-rebase [-i] (--continue | --abort | --skip)
>>> +git-rebase [-i] [-m] (--continue | --abort | --skip)
>>
>> Again, dashless. And I'd not mention the useless -i here, the man page
>> doesn't either:
>>
>> -git-rebase [-i] (--continue | --abort | --skip)
>> +git rebase (--continue | --abort | --skip)
>>
>
> It was already there, so I didn't consider it, but I guess it makes
> sense. Besides, I aimed at not loosing any information while making it
> a bit clearer.
>
>> These two items are misplaced in the help (I think). They're not like
>> abort, continue, skip, but then, the man page doesn't group those
>> separately either.
>>
>> +no-verify          override pre-rebase hook from stopping the operation
>> +root               rebase all reachable commmits up to the root(s)
>>
>
> Agree.
>
>>>  Actions:
>>>  continue           continue rebasing process
>>>  abort              abort rebasing process and restore original branch
>>
>> As above, remove the next two lines after your patch:
>>
>> -no-verify          override pre-rebase hook from stopping the operation
>> -root               rebase all reachable commmits up to the root(s)
>
> I don't follow this. Are you repeating yourself now? :)

Yes :) ... was just finishing off moving those two lines.

Cheers,
Baz

>
> --
> Erik "kusma" Faye-Lund
>