Re: possible usability issue in rebase -i?

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

2009/10/27 Erik Faye-Lund <kusmabite@googlemail.com>:
> On Tue, Oct 27, 2009 at 4:17 PM, Baz <brian.ewins@gmail.com> wrote:
>> I've got a somewhat related minor usability issue with rebase -i. I
>> accidentally typed something like 'git rebase -i -z' and got this
>> message:
>>
>> error: unknown switch `z'
>> usage: git-rebase [-i] [options] [--] <upstream> [<branch>]
>>   or: git-rebase [-i] (--continue | --abort | --skip)
>>
>> Available options are
>>    -v, --verbose         display a diffstat of what changed upstream
>>    --onto ...            rebase onto given branch instead of upstream
>>    -p, --preserve-merges
>>                          try to recreate merges instead of ignoring them
>>    -s, --strategy ...    use the given merge strategy
>>    -m, --merge           always used (no-op)
>>    -i, --interactive     always used (no-op)
>>
>> The last two lines were the surprise. It suggested to me that '-i' and
>> '-m' were now the defaults for git-rebase - which of course they're
>> not. A user would not know that this is actually reporting the flags
>> that work for git-rebase--interactive, especially since that's not
>> what the command calls itself. I wasn't sure about the best approach
>> to fixing this - the only comparable commands that pass arbitrary
>> flags down to an exec'd program make it clear what program is going to
>> be called (usually git merge) and so interpreting errors is easier.
>>
>> It seems the intent here was to signal that the flags are different
>> once a rebase is in progress, but this usage message is shown when
>> rebase -i -z is called in any state.
>
> If that is the case, my instinct tells me that this information should
> be reflected in the usage-string (instead of the parameter
> description). Something like this?

I'm fine with this way of fixing it, but I'd make a few more changes...

>
> --->8---
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 23ded48..3ed5f94 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -13,15 +13,15 @@
>  OPTIONS_KEEPDASHDASH=
>  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>]


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

>  --
>  Available options are
>  v,verbose          display a diffstat of what changed upstream
>  onto=              rebase onto given branch instead of upstream
>  p,preserve-merges  try to recreate merges instead of ignoring them
>  s,strategy=        use the given merge strategy
> -m,merge            always used (no-op)
> -i,interactive      always used (no-op)
> +m,merge            use merging strategies
> +i,interactive      interactively edit commits

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)

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

-Baz

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