builtin/show-ref.c: support `--count` for limiting output

Documentation/git-show-ref.txt +6 -0

+	if (max_count) {
+		int compatible = 0;
+
+		if (max_count < 0)
+			error(_("invalid --count argument: (`%d' < 0)"),
+			      max_count);

Documentation/git-show-ref.txt +2 -0

+	}
+
 	if (exclude_arg)
 		return exclude_existing(exclude_existing_arg);

t/t1403-show-ref.sh +0 -0

index 9252a581ab..b79e114c1e 100755
@@ -196,4 +196,25 @@ test_expect_success 'show-ref --verify with dangling ref' '
 	)

Documentation/git-show-ref.txt SYNOPSIS · line 13

Quoted 10 lines · Taylor Blau

Documentation/git-show-ref.txt +1 -1

index ab4d271925..28256c04dd 100644
@@ -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> ]

Quoted 6 lines

Documentation/git-show-ref.txt +6 -0

+	if (max_count) {
+		int compatible = 0;
+
+		if (max_count < 0)
+			error(_("invalid --count argument: (`%d' < 0)"),
+			      max_count);

Documentation/git-show-ref.txt SYNOPSIS

Quoted 10 lines

Documentation/git-show-ref.txt +10 -0

+		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.

Quoted 11 lines

Documentation/git-show-ref.txt +2 -0

+	}
+
 	if (exclude_arg)
 		return exclude_existing(exclude_existing_arg);

t/t1403-show-ref.sh +0 -0

index 9252a581ab..b79e114c1e 100755
@@ -196,4 +196,25 @@ test_expect_success 'show-ref --verify with dangling ref' '
 	)

t/t1403-show-ref.sh test_expect_success 'show-ref --verify with dangling ref' ' · line 206

Quoted 10 lines

t/t1403-show-ref.sh +8 -0

 '
 
+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...

t/t1403-show-ref.sh test_expect_success 'show-ref --verify with dangling ref' ' · line 211

Quoted 5 lines

t/t1403-show-ref.sh +5 -0

+'
+
+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?

t/t1403-show-ref.sh test_expect_success 'show-ref --verify with dangling ref' ' · line 217

Quoted 4 lines

t/t1403-show-ref.sh +4 -0

+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.

Quoted 18 lines · Ævar Arnfjörð Bjarmason

Documentation/git-show-ref.txt SYNOPSIS · line 13

Quoted 10 lines · Taylor Blau

Documentation/git-show-ref.txt +1 -1

index ab4d271925..28256c04dd 100644
@@ -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.:

Quoted 1 line

	[ [--verify] [--exclude-existing] | --count=<n> ]

Good catch. Should this be squashed into the first example in the SYNOPSIS, the second, or a new one?

Quoted 19 lines

Quoted 6 lines

Documentation/git-show-ref.txt +6 -0

+	if (max_count) {
+		int compatible = 0;
+
+		if (max_count < 0)
+			error(_("invalid --count argument: (`%d' < 0)"),
+			      max_count);

Documentation/git-show-ref.txt SYNOPSIS

Quoted 10 lines

Documentation/git-show-ref.txt +10 -0

+		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().

Quoted 3 lines

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.

Good catch again. I wasn't aware of usage_msg_optf(), but it's exactly what I'm looking for here. It does mean that we'd only print one warning at a time, but I think that's a fair tradeoff, and unlikely to matter in practice anyways.

And I must have dropped the parameterized msgids on the floor when preparing this patch, since I definitely have it locally. Oops, fixed.

Quoted 24 lines

Quoted 11 lines

Documentation/git-show-ref.txt +2 -0

+	}
+
 	if (exclude_arg)
 		return exclude_existing(exclude_existing_arg);

t/t1403-show-ref.sh +0 -0

index 9252a581ab..b79e114c1e 100755
@@ -196,4 +196,25 @@ test_expect_success 'show-ref --verify with dangling ref' '
 	)

t/t1403-show-ref.sh test_expect_success 'show-ref --verify with dangling ref' ' · line 206

Quoted 10 lines

t/t1403-show-ref.sh +8 -0

 '
 
+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...

Quoted 8 lines

t/t1403-show-ref.sh test_expect_success 'show-ref --verify with dangling ref' ' · line 211

Quoted 5 lines

t/t1403-show-ref.sh +5 -0

+'
+
+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?

Muscle memory ;).

Quoted 9 lines

t/t1403-show-ref.sh test_expect_success 'show-ref --verify with dangling ref' ' · line 217

Quoted 4 lines

t/t1403-show-ref.sh +4 -0

+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.

I like having the extra test to ensure the error we got made sense, but I agree either would work. I modified the grep expressions to replace leading "."'s with "[-]", and "."'s in the middle of the expression with "-".

Quoted 19 lines · Taylor Blau

Quoted 18 lines · Ævar Arnfjörð Bjarmason

Documentation/git-show-ref.txt SYNOPSIS · line 13

Quoted 10 lines · Taylor Blau

Documentation/git-show-ref.txt +1 -1

index ab4d271925..28256c04dd 100644
@@ -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.:

Quoted 4 lines

Quoted 1 line

	[ [--verify] [--exclude-existing] | --count=<n> ]

Good catch. Should this be squashed into the first example in the SYNOPSIS, the second, or a new one?

Personally I really don't care if the end-state is good :)

This short patch series adds support for a new --count argument for limiting the output of show-ref (à-la the for-each-ref option by the same name).

This is useful in contexts where a caller wants to avoid enumerating more references than necessary (e.g., they only care whether a tag exists, but not how many or what they are called) but doesn't have control of the output stream (e.g., they are in Ruby and can't pipe the output to head -n 1).

Quoted 2 lines · Taylor Blau

This short patch series adds support for a new --count argument for limiting the output of show-ref (à-la the for-each-ref option by the same name).

It makes me wonder why we limit this to show-ref.

    $ git --pipe-to-head-N=3 any-command args...

IOW, having to add an option like this feels absurd.

Quoted 4 lines

This is useful in contexts where a caller wants to avoid enumerating more references than necessary (e.g., they only care whether a tag exists, but not how many or what they are called) but doesn't have control of the output stream (e.g., they are in Ruby and can't pipe the output to head -n 1).

Are you saying that Ruby is incapable of run a command line like

   av[0] = "sh"
   av[1] = "-c"
   av[2] = "git show-ref blah | head -n 1"
   av[3] = NULL

If you're strongly opposed to having show-ref match for-each-ref's --count option, I won't be too sad. But I'm not in a huge rush to replace this series with git --pipe-to-head-N=<n> either, FWIW.

Quoted 3 lines · Taylor Blau

If you're strongly opposed to having show-ref match for-each-ref's --count option, I won't be too sad. But I'm not in a huge rush to replace this series with git --pipe-to-head-N=<n> either, FWIW.

Heh, to me "git --pipe-to-head-N=<n>" smells equally absurd, too ;-)

Quoted 7 lines · Junio C Hamano

Quoted 2 lines · Taylor Blau

This short patch series adds support for a new --count argument for limiting the output of show-ref (à-la the for-each-ref option by the same name).

It makes me wonder why we limit this to show-ref.

Quoted 3 lines

    $ git --pipe-to-head-N=3 any-command args...

IOW, having to add an option like this feels absurd.

I don't disagree. But --pipe-to-head-N=3 feels like too broad a stroke. This series at least imitates for-each-ref's --count option, which makes it feel acceptable to me (if not a little silly).

Quoted 7 lines

Quoted 4 lines

This is useful in contexts where a caller wants to avoid enumerating more references than necessary (e.g., they only care whether a tag exists, but not how many or what they are called) but doesn't have control of the output stream (e.g., they are in Ruby and can't pipe the output to head -n 1).

Are you saying that Ruby is incapable of run a command line like

Quoted 4 lines

   av[0] = "sh"
   av[1] = "-c"
   av[2] = "git show-ref blah | head -n 1"
   av[3] = NULL

No, Ruby is perfectly capable of doing that. But it involves an extra process (two, if head isn't a shell builtin) [...]

[...]and the additional
overhead of creating a pipe and passing data through it instead of writing directly to stdout.

Quoted 8 lines · Taylor Blau

Quoted 7 lines · Junio C Hamano

Quoted 2 lines · Taylor Blau

This short patch series adds support for a new --count argument for limiting the output of show-ref (à-la the for-each-ref option by the same name).

It makes me wonder why we limit this to show-ref.

Quoted 7 lines

Quoted 3 lines

    $ git --pipe-to-head-N=3 any-command args...

IOW, having to add an option like this feels absurd.

I don't disagree. But --pipe-to-head-N=3 feels like too broad a stroke. This series at least imitates for-each-ref's --count option, which makes it feel acceptable to me (if not a little silly).

Yeah, although I do think it's worthwhile to think about where certain UX decisions are leading us, i.e. the logical conclusion here is to have every command that emits >1 lines support --count, which as your patch here shows needs special support, and even in your case you haven't implemented it in a way that's compatible with all existing options.

B.t.w. why would a --count for --verify not just by supported have these be equivalent:

    # same
    git tag --count=3 --verify <name>
    git tag --verify <name> | head -n 3

Quoted 7 lines

Quoted 7 lines

Quoted 4 lines

This is useful in contexts where a caller wants to avoid enumerating more references than necessary (e.g., they only care whether a tag exists, but not how many or what they are called) but doesn't have control of the output stream (e.g., they are in Ruby and can't pipe the output to head -n 1).

Are you saying that Ruby is incapable of run a command line like

Quoted 7 lines

Quoted 4 lines

   av[0] = "sh"
   av[1] = "-c"
   av[2] = "git show-ref blah | head -n 1"
   av[3] = NULL

No, Ruby is perfectly capable of doing that. But it involves an extra process (two, if head isn't a shell builtin) [...]

Maybe this really is a limitation of ruby, or maybe I'm missing something, but doesn't it support just opening a process without "sh -c" and piping the output to your current process, as this perl command which makes use of execve() will do:

    $ perl -Mautodie=:all -wE '
        my $i = 0; my $lim = shift;
        open my $fh, "-|", qw(git show-ref master);
        while (<$fh>) {
            last if $i++ >= $lim;
            print "$.: $_";
        };' 10

Some quick searching for docs online suggests that Ruby's Open3 and/or Process.spawn might be the equivalent.

Isn't that something that would make this workaround unnecessary? Well, maybe not because...

Quoted 3 lines

[...]and the additional
overhead of creating a pipe and passing data through it instead of writing directly to stdout.

It wouldn't take care of this part, but I'm struggling to think of cases where you'd be running this in the context of github.com and not already need to capture the output of the command. I.e. surely you're already piping stdout/stderr into your program, no?