2009/10/28 Erik Faye-Lund <kusmabite@xxxxxxxxxxxxxx>: > On Tue, Oct 27, 2009 at 10:05 PM, Baz <brian.ewins@xxxxxxxxx> 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] [--] <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. > >> >>> -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 > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html