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@xxxxxx> 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@xxxxxxxxxxxxxx/ > > [2]: https://lore.kernel.org/git/CAPig+cRTOMie0rUf=Mhbo9e2EXf-_2kQyMeqpB9OCRB1MZZ1rw@xxxxxxxxxxxxxx/ > > [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@xxxxxxxxxxxxxx> > Signed-off-by: René Scharfe <l.s.r@xxxxxx> > --- > 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:
signature.asc
Description: PGP signature
