Re: [PATCH] switch: fix errors and comments related to -c and -C

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Apr 29, 2020, 16:31 UTC
Message-ID: <CAPig+cS-f7JOkhW7yf_h4bXx75Y_=_0e7uPTCfkBCHvc8WSUBw@mail.gmail.com>
In-Reply-To: <20200429160948.GB83442@syl.local>

On Wed, Apr 29, 2020 at 12:10 PM Taylor Blau <me@ttaylorr.com> wrote:

Show 6 quoted lines

> On Wed, Apr 29, 2020 at 05:00:19AM -0400, Denton Liu wrote:
> > In d787d311db (checkout: split part of it to new command 'switch',
> > 2019-03-29), the `git switch` command was created by extracting the
> > common functionality of cmd_checkout() in checkout_main(). However, in
> > b7b5fce270 (switch: better names for -b and -B, 2019-03-29), these
> > the branch creation and force creation options for 'switch' were changed

s/these the/the/

Show 12 quoted lines

> > to -c and -C, respectively. As a result of this, error messages and
> > comments that previously referred to `-b` and `-B` became invalid for
> > `git switch`.
> >
> > For comments that refer to `-b` and `-B`, add `-c` and `-C` to the
> > comment.
>
> I had to read this sentence a couple of times more than I would have
> liked to in order to grok it fully. Would it be perhaps clearer as:
>
>   Update comments in 'cmd_checkout()' that mention `-b` or `-B` to
>   instead refer to `-c` or `-C` when invoked from 'git switch'.

I had no problem groking Denton's wording but had to re-read this proposal several times before understanding it. I could try providing yet another proposal, however, I think the entire sentence can simply be dropped (after all, it's just stating the obvious).

Show 15 quoted lines

> > +             die(variant == CMD_CHECKOUT ?
> > +                             _("-b, -B and --orphan are mutually exclusive") :
> > +                             _("-c, -C and --orphan are mutually exclusive"));
>
> Hmm. Do we need to generate an extra string for translation here? If the
> string was instead:
>
>   _("%s and --orphan are mutually exclusive")
>
> where the first format string is filled in something like:
>
>   die(_("%s and --orphan are mutually exclusive"),
>       variant == CMD_CHECKOUT ? "-b, -B" : "-c, -C");
>
> may save translators some work.

We don't know the grammatical or syntactic rules of each language, so hard-coding untranslatable "-b, -B" is contraindicated. Since the option letters ought not be translated (just as "--orphan" shouldn't be), the letters themselves could be interpolated into the string. However, that's probably less helpful for translators since it eliminates contextual clues. Therefore, it seems like a good idea to leave it as two separate translatable strings (as the patch already does).