On Mon, Jun 06 2022, Taylor Blau wrote:
> diff --git a/Documentation/git-show-ref.txt b/Documentation/git-show-ref.txt
> index ab4d271925..28256c04dd 100644
> --- a/Documentation/git-show-ref.txt
> +++ b/Documentation/git-show-ref.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
> [verse]
> 'git show-ref' [-q|--quiet] [--verify] [--head] [-d|--dereference]
> [-s|--hash[=<n>]] [--abbrev[=<n>]] [--tags]
> - [--heads] [--] [<pattern>...]
> + [--heads] [--count=<n>] [--] [<pattern>...]
In addition to what Junio noted, the SYNOPSIS is now inaccurate per your
documentation. I.e. if this option is incompatible with --verify and
--exclude-existing we should use "|" to indicate that, e.g.:
[ [--verify] [--exclude-existing] | --count=<n> ]
> + if (max_count) {
> + int compatible = 0;
> +
> + if (max_count < 0)
> + error(_("invalid --count argument: (`%d' < 0)"),
> + max_count);
> + else if (verify)
> + error(_("--count is incompatible with %s"), "--verify");
> + else if (exclude_arg)
> + error(_("--count is incompatible with %s"),
> + "--exclude-existing");
> + else
> + compatible = 1;
> +
> + if (!compatible)
> + usage_with_options(show_ref_usage, show_ref_options);
Instead of this "int compatible" and if/else-if" just use usage_msg_optf().
That or die_for_incompatible_opt4(), at least the new _() messages
should make use of the same translations. I.e. we recently made these
parameterized.
> + }
> +
> if (exclude_arg)
> return exclude_existing(exclude_existing_arg);
>
> diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
> index 9252a581ab..b79e114c1e 100755
> --- a/t/t1403-show-ref.sh
> +++ b/t/t1403-show-ref.sh
> @@ -196,4 +196,25 @@ test_expect_success 'show-ref --verify with dangling ref' '
> )
> '
>
> +test_expect_success 'show-ref --count limits relevant output' '
> + git show-ref --heads --count=1 >out &&
> + test_line_count = 1 out
> +'
> +
> +test_expect_success 'show-ref --count rejects invalid input' '
> + test_must_fail git show-ref --count=-1 2>err &&
> + grep "invalid ..count argument: (.-1. < 0)" err
The use of .. here seems odd...
> +'
> +
> +test_expect_success 'show-ref --count incompatible with --verify' '
> + test_must_fail git show-ref --count=1 --verify HEAD 2>err &&
> + grep "..count is incompatible with ..verify" err
...i.e. this looks like a way to avoid "--" at the beginning, but then
why use it in the middle of the regex?
> +test_expect_success 'show-ref --count incompatible with --exclude-existing' '
> + echo "refs/heads/main" >in &&
> + test_must_fail git show-ref --count=1 --exclude-existing <in 2>err &&
> + grep "..count is incompatible with ..exclude.existing" err
Seems like you could avoid it entirely by escaping it, e.g. "[-]-" at
the beginning, or in this case I think "test_expect_code 129" would be
more than sufficient, we use that to test "had usage spewed at us"
elsewhere.
