Re: [PATCH] i18n: factorize even more 'incompatible options' messages
From: Patrick Steinhardt <hidden>
Date: 2023-11-28 12:33:51
On Mon, Nov 27, 2023 at 06:39:41PM +0100, René Scharfe wrote:
Am 26.11.23 um 18:49 schrieb Eric Sunshine:quoted
On Sun, Nov 26, 2023 at 6:57 AM René Scharfe [off-list ref] wrote:quoted
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
quoted hunk ↗ jump to hunk
quoted
[1]: https://lore.kernel.org/git/CAPig+cSrp7vZuy7D_ENHKZKZzF4OSmCtfYNHPGMtS1Hj6gArDw@mail.gmail.com/ (local) [2]: https://lore.kernel.org/git/CAPig+cRTOMie0rUf=Mhbo9e2EXf-_2kQyMeqpB9OCRB1MZZ1rw@mail.gmail.com/ (local) [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 <redacted> Signed-off-by: René Scharfe <redacted> --- 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
Attachments
- signature.asc [application/pgp-signature] 833 bytes