From: Erik Faye-Lund <kusmabite@googlemail.com>
Date: Wed, 28 Oct 2009 12:20:03 GMT
Subject: Re: possible usability issue in rebase -i?
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.

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

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

-- 
Erik "kusma" Faye-Lund