From: Baz Date: Wed, 28 Oct 2009 14:34:09 GMT Subject: Re: possible usability issue in rebase -i? Message-ID: <2faad3050910280734l7297c30erfb0a47b12b0bd07d@mail.gmail.com> In-Reply-To: <40aa078e0910280520t497f1289sf374a3a501856a23@mail.gmail.com> 2009/10/28 Erik Faye-Lund : > On Tue, Oct 27, 2009 at 10:05 PM, Baz 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. > 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. > >>>  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 >> [--] [] >> > > 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. > >> >>> -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? :) Yes :) ... was just finishing off moving those two lines. Cheers, Baz > > -- > Erik "kusma" Faye-Lund >