Subject: Re: [PATCH v2 11/11] i18n: turn even more messages into "cannot be used together" ones

From: Jean-Noël AVILA <[email-masked]>
Date: Wed, 29 Dec 2021 16:49:53 +0100

On Tuesday, 28 December 2021 08:33:15 CET Johannes Sixt wrote:
> Am 28.12.21 um 00:23 schrieb Jean-Noël Avila via GitGitGadget:
> > From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <[email-masked]>
> > diff --git a/builtin/am.c b/builtin/am.c
> > index 8677ea2348a..be0e32f69cf 100644
> > --- a/builtin/am.c
> > +++ b/builtin/am.c
> > @@ -2230,8 +2230,8 @@ static int parse_opt_show_current_patch(const struct option *opt, const char *ar
> >  	}
> >  
> >  	if (resume->mode == RESUME_SHOW_PATCH && new_value != resume->sub_mode)
> > -		return error(_("--show-current-patch=%s is incompatible with "
> > -			       "--show-current-patch=%s"),
> > +		return error(_("options '--show-current-patch=%s' and '--show-current-patch=%s' "
> > +					   "cannot used together"),
> 
> Missing "be".

Thanks.

> 
> >  			     arg, valid_modes[resume->sub_mode]);
> >  
> >  	resume->mode = RESUME_SHOW_PATCH;
> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> > index 86fc03242b8..400ceaba5e8 100644
> > --- a/builtin/cat-file.c
> > +++ b/builtin/cat-file.c
> > @@ -729,7 +729,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
> >  	}
> >  
> >  	if (force_path && batch.enabled) {
> > -		error("--path=<path> incompatible with --batch");
> > +		error("'--path=<path>' and '--batch' cannot be used together");
> 
> Missing "options" perhaps.
>

Thanks.
 
> >  		usage_with_options(cat_file_usage, options);
> >  	}
> >  
> > diff --git a/builtin/describe.c b/builtin/describe.c
> > index fd5ba1fc604..130b87b3a34 100644
> > --- a/builtin/describe.c
> > +++ b/builtin/describe.c
> > @@ -670,9 +670,9 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
> >  		}
> >  		describe("HEAD", 1);
> >  	} else if (dirty) {
> > -		die(_("--dirty is incompatible with commit-ishes"));
> > +		die(_("'%s' and commit-ishes cannot be used together"), "--dirty");
> >  	} else if (broken) {
> > -		die(_("--broken is incompatible with commit-ishes"));
> > +		die(_("'%s' and  commit-ishes cannot be used together"), "--broken");
> 
> I noticed immediately that the two texts are not the same because they
> are not aligned, but it took me a few seconds to notice the extra blank
> in the second one.

Thanks.

> 
> >  	} else {
> >  		while (argc-- > 0)
> >  			describe(*argv++, argc == 0);
> > diff --git a/builtin/init-db.c b/builtin/init-db.c
> > index 546f9c595e7..b85dffbdf5c 100644
> > --- a/builtin/init-db.c
> > +++ b/builtin/init-db.c
> > @@ -681,7 +681,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
> >  	}
> >  	else {
> >  		if (real_git_dir)
> > -			die(_("--separate-git-dir incompatible with bare repository"));
> > +			die(_("'--separate-git-dir' and bare repository cannot be used together"));
> 
> IMO, in this case, the original text is awkward, and the transformation
> is worse because it really sounds like a mechanical replacement. I
> suggest not to do this transformation.

Agreed.

> >  
> >  test_fixup_reword_opt () {
> >  	test_expect_success "--fixup=reword: incompatible with $1" "
> > -		echo 'fatal: reword option of --fixup is mutually exclusive with'\
> > -			'--patch/--interactive/--all/--include/--only' >expect &&
> > +		echo 'fatal: reword option of --fixup and'\
> > +			'--patch/--interactive/--all/--include/--only'\
> > +			'cannot be used together' >expect &&
> 
> On first sight, it looked significant that there are no blanks between
> "and", the single-quote, and the backslash. On second thought, the
> backslash is just a line continuation, and the blank that is needed in
> the text is inserted by "echo" because it emits its three arguments, the
> single-quoted strings', in blank-separated form. This means that there
> must not be a blank after "and" and after "--only" and before the
> single-quotes. There could be a blank before the backslash.
> 
> Good.

That's exactly that. I struggled for quite some time when trying to split the 
string and the test was failing. Very counter-intuitive, even after you understand why.

Thanks for your review and spotting the mistakes. Will reroll.

As a side note, while editing all these tests in series, i could not help but notice that there 
are different styles of writing them.