Re: [PATCH] rev-list: make empty --stdin not an error

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> Instead, let's keep two flags: one to denote when we got
> actual input (which triggers both roles) and one for when we
> read stdin (which triggers only the first).
>
> This does mean a caller interested in the first role has to
> check both flags, but there's only one such caller. And any
> future callers might want to make the distinction anyway
> (e.g., if they care less about erroring out, and more about
> whether revision.c soaked up our stdin).
>
> [1] https://public-inbox.org/git/20170802223416.gwiezhbuxbdmbjzx@sigill.intra.peff.net/
>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  builtin/rev-list.c       | 2 +-
>  revision.c               | 1 +
>  revision.h               | 5 +++++
>  t/t6018-rev-list-glob.sh | 2 +-
>  4 files changed, 8 insertions(+), 2 deletions(-)

I think this makes sense, but if we were to give a dedicated field
in the revs structure, can we lose the local variable at the same
time, I wonder?

Thanks.

> diff --git a/revision.c b/revision.c
> index de4dce600d..4d53102cf4 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2369,6 +2369,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>  				}
>  				if (read_from_stdin++)
>  					die("--stdin given twice?");
> +				revs->read_from_stdin = 1;
>  				read_revisions_from_stdin(revs, &prune_data);
>  				continue;
>  			}
> diff --git a/revision.h b/revision.h
> index 007278cc11..1225957927 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -82,6 +82,11 @@ struct rev_info {
>  	 */
>  	int rev_input_given;
>  
> +	/*
> +	 * Whether we read from stdin due to the --stdin option.
> +	 */
> +	int read_from_stdin;
> +
>  	/* topo-sort */
>  	enum rev_sort_order sort_order;
>  
> diff --git a/t/t6018-rev-list-glob.sh b/t/t6018-rev-list-glob.sh
> index 0bf10d0686..db8a7834d8 100755
> --- a/t/t6018-rev-list-glob.sh
> +++ b/t/t6018-rev-list-glob.sh
> @@ -255,7 +255,7 @@ test_expect_success 'rev-list accumulates multiple --exclude' '
>  	compare rev-list "--exclude=refs/remotes/* --exclude=refs/tags/* --all" --branches
>  '
>  
> -test_expect_failure 'rev-list should succeed with empty output on empty stdin' '
> +test_expect_success 'rev-list should succeed with empty output on empty stdin' '
>  	git rev-list --stdin </dev/null >actual &&
>  	test_must_be_empty actual
>  '