Re: [PATCH] i18n: factorize even more 'incompatible options' messages

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 4338 bytes --]

On Mon, Nov 27, 2023 at 06:39:41PM +0100, René Scharfe wrote:
> Am 26.11.23 um 18:49 schrieb Eric Sunshine:
> > On Sun, Nov 26, 2023 at 6:57 AM René Scharfe <l.s.r@web.de> wrote:
> >> Continue the work of 12909b6b8a (i18n: turn "options are incompatible"
> >> into "cannot be used together", 2022-01-05) and a699367bb8 (i18n:
> >> factorize more 'incompatible options' messages, 2022-01-31) to use the
> >> same parameterized error message for reporting incompatible command line
> >> options.  This reduces the number of strings to translate and makes the
> >> UI slightly more consistent.
> >
> > Thanks for tackling this.
> >
> > A couple additional instances recently slipped into `show-ref.c` which
> > were caught during review[1,2] but nevertheless made it to
> > "master"[3,4]. This patch, of course, doesn't need to address those,
> > but if rerolling for some other reason, perhaps they can be included,
> > as well(?).

Ah, I wasn't aware of these new wrappers, either. The below patch looks
good to me, thanks for the fixup.

Patrick

> > [1]: https://lore.kernel.org/git/CAPig+cSrp7vZuy7D_ENHKZKZzF4OSmCtfYNHPGMtS1Hj6gArDw@mail.gmail.com/
> > [2]: https://lore.kernel.org/git/CAPig+cRTOMie0rUf=Mhbo9e2EXf-_2kQyMeqpB9OCRB1MZZ1rw@mail.gmail.com/
> > [3]: 199970e72f (builtin/show-ref: ensure mutual exclusiveness of
> > subcommands, 2023-10-31)
> > [4]: 9080a7f178 (builtin/show-ref: add new mode to check for reference
> > existence, 2023-10-31)
> 
> [4] changes the message added by [3], so that's one instance, right?
> 
> --- >8 ---
> Subject: [PATCH] show-ref: use die_for_incompatible_opt3()
> 
> Use the standard message for reporting the use of multiple mutually
> exclusive options by calling die_for_incompatible_opt3() instead of
> rolling our own.  This has the benefits of showing only the actually
> given options, reducing the number of strings to translate and making
> the UI slightly more consistent.
> 
> Adjust the test to no longer insist on a specific order of the
> reported options, as this implementation detail does not affect the
> usefulness of the error message.
> 
> Reported-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  builtin/show-ref.c  |  6 +++---
>  t/t1403-show-ref.sh | 16 +++++++++-------
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/builtin/show-ref.c b/builtin/show-ref.c
> index 7aac525a87..59d2291cbf 100644
> --- a/builtin/show-ref.c
> +++ b/builtin/show-ref.c
> @@ -315,9 +315,9 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
>  	argc = parse_options(argc, argv, prefix, show_ref_options,
>  			     show_ref_usage, 0);
> 
> -	if ((!!exclude_existing_opts.enabled + !!verify + !!exists) > 1)
> -		die(_("only one of '%s', '%s' or '%s' can be given"),
> -		    "--exclude-existing", "--verify", "--exists");
> +	die_for_incompatible_opt3(exclude_existing_opts.enabled, "--exclude-existing",
> +				  verify, "--verify",
> +				  exists, "--exists");
> 
>  	if (exclude_existing_opts.enabled)
>  		return cmd_show_ref__exclude_existing(&exclude_existing_opts);
> diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
> index b50ae6fcf1..d477689e33 100755
> --- a/t/t1403-show-ref.sh
> +++ b/t/t1403-show-ref.sh
> @@ -197,18 +197,20 @@ test_expect_success 'show-ref --verify with dangling ref' '
>  '
> 
>  test_expect_success 'show-ref sub-modes are mutually exclusive' '
> -	cat >expect <<-EOF &&
> -	fatal: only one of ${SQ}--exclude-existing${SQ}, ${SQ}--verify${SQ} or ${SQ}--exists${SQ} can be given
> -	EOF
> -
>  	test_must_fail git show-ref --verify --exclude-existing 2>err &&
> -	test_cmp expect err &&
> +	grep "verify" err &&
> +	grep "exclude-existing" err &&
> +	grep "cannot be used together" err &&
> 
>  	test_must_fail git show-ref --verify --exists 2>err &&
> -	test_cmp expect err &&
> +	grep "verify" err &&
> +	grep "exists" err &&
> +	grep "cannot be used together" err &&
> 
>  	test_must_fail git show-ref --exclude-existing --exists 2>err &&
> -	test_cmp expect err
> +	grep "exclude-existing" err &&
> +	grep "exists" err &&
> +	grep "cannot be used together" err
>  '
> 
>  test_expect_success '--exists with existing reference' '
> --
> 2.43.0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]