From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Fri, 04 May 2018 06:52:37 GMT
Subject: Re: [PATCH 02/18] Add a new builtin: branch-diff
Message-ID: <nycvar.QRO.7.76.6.1805040843050.77@tvgsbejvaqbjf.bet>
In-Reply-To: <CAPig+cQv7tNCNhDdThhhDYEE=XmB0xO35Qjvpw+-MgCg0W3ovQ@mail.gmail.com>

Hi Eric,

On Thu, 3 May 2018, Eric Sunshine wrote:

> On Thu, May 3, 2018 at 11:30 AM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
> > This builtin does not do a whole lot so far, apart from showing a usage
> > that is oddly similar to that of `git tbdiff`. And for a good reason:
> > the next commits will turn `branch-diff` into a full-blown replacement
> > for `tbdiff`.
> >
> > At this point, we ignore tbdiff's color options, as they will all be
> > implemented later and require some patches to the diff machinery.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > diff --git a/builtin/branch-diff.c b/builtin/branch-diff.c
> > @@ -0,0 +1,40 @@
> > +static const char * const builtin_branch_diff_usage[] = {
> > +       N_("git rebase--helper [<options>] ( A..B C..D | A...B | base A B )"),
> > +       NULL
> > +};
> 
> The formatting of "<options>" vs. "base" confused me into thinking
> that the latter was a literal keyword, but I see from reading patch
> 3/18 that it is not a literal at all, thus probably ought to be
> specified as "<base>".

Good point. Or maybe BASE?

Or I should just use the same convention as in the man page. Or not, as
the usage should be conciser.

This is what I have currently:

static const char * const builtin_branch_diff_usage[] = {
N_("git branch-diff [<options>] <old-base>..<old-tip> <new-base>..<new-tip>"),
N_("git branch-diff [<options>] <old-tip>...<new-tip>"),
N_("git branch-diff [<options>] <base> <old-tip> <new-tip>"),
NULL
};

Thanks,
Dscho