Subject: Re: [PATCH v3 2/4] revision.h: refactor "disable_stdin" and "read_from_stdin"

From: Junio C Hamano <[email-masked]>
Date: Thu, 08 Jul 2021 15:10:12 -0700

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> Change the two "disable_stdin" and "read_from_stdin" flags to an enum,
> in preparation for a subsequent commit adding more flags.
>
> The interaction between these is more subtle than they might appear at
> first sight, as noted in a12cbe23ef7. "read_stdin" is not the inverse
> of "disable_stdin", rather we read stdin if we see the "--stdin"
> option.
>
> The "read" is intended to understood as "I read it", not "you should

"I've read it already" would have been easier to understand.

>  			if (!strcmp(arg, "--stdin")) {
> -				if (revs->disable_stdin) {
> +				switch (revs->stdin_handling) {
> +				case REV_INFO_STDIN_IGNORE:
>  					argv[left++] = arg;
>  					continue;
> +				case REV_INFO_STDIN_CONSUME_ON_OPTION:
> +					if (revs->consumed_stdin++)
> +						die("--stdin given twice?");
> +					read_revisions_from_stdin(revs, &prune_data);
> +					continue;
>  				}

More importantly, consumed_stdin will start at zero and if it is
found to be non-zero here, it is an immediate error, hence ...

> +	/*
> +	 * Did we read from stdin due to stdin_handling ==
> +	 * REV_INFO_STDIN_CONSUME_ON_OPTION and seeing the --stdin
> +	 * option?
>  	 */
> -	int read_from_stdin;
> +	int consumed_stdin;

... it does not have to be a full integer.
Just a single-bit would do.

But none of the above is grave enough to require an update.