From: Baz Date: Tue, 27 Oct 2009 21:05:53 GMT Subject: Re: possible usability issue in rebase -i? Message-ID: <2faad3050910271405k4a391184vb978b9b35484383b@mail.gmail.com> In-Reply-To: <40aa078e0910270850u6ffec41cj372da11d9df533f@mail.gmail.com> 2009/10/27 Erik Faye-Lund : > On Tue, Oct 27, 2009 at 4:17 PM, Baz 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] [--] [] >>   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] [--] [] 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] [--] [] +git rebase [--interactive | -i] [options] [--onto ] [--] [] +git rebase [--interactive | -i] [options] --onto --root [--] [] > -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 >