Re: possible usability issue in rebase -i?

2009/10/28 Erik Faye-Lund [off-list ref]:

On Tue, Oct 27, 2009 at 10:05 PM, Baz [off-list ref] wrote:

quoted

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.

quoted

quoted

 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.

quoted

quoted

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

quoted

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.

quoted

quoted

 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