Re: possible usability issue in rebase -i?

From: Erik Faye-Lund <kusmabite@googlemail.com>
Date: Oct 28, 2009, 12:20 UTC
Message-ID: <40aa078e0910280520t497f1289sf374a3a501856a23@mail.gmail.com>
In-Reply-To: <2faad3050910271405k4a391184vb978b9b35484383b@mail.gmail.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.

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.

Show 13 quoted lines

>>  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".

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.

Show 10 quoted lines

>
>> -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.

Show 7 quoted lines

> 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.

Show 8 quoted lines

>>  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? :)

-- 
Erik "kusma" Faye-Lund