Re: [PATCH v2 2/4] revision.h: unify "disable_stdin" and "read_from_stdin"
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Jun 18, 2021, 17:54 UTC
- Message-ID
- <20210618175443.773403-1-jonathantanmy@google.com>
- In-Reply-To
- <patch-2.4-d88b2c0410-20210617T105537Z-avarab@gmail.com>
About the subject:
> revision.h: unify "disable_stdin" and "read_from_stdin"
Hmm...there is no unification, I think? Both of these remain distinct - it's just that both were renamed and one was changed into an enum.
Show 14 quoted lines
> @@ -114,9 +119,22 @@ struct rev_info {
> int rev_input_given;
>
> /*
> - * Whether we read from stdin due to the --stdin option.
> + * How should we handle seeing --stdin?
> + *
> + * Defaults to reading if we see it with
> + * REV_INFO_STDIN_CONSUME_ON_OPTION.
> + *
> + * Can be set to REV_INFO_STDIN_IGNORE to ignore any provided
> + * --stdin option.
> + */
> + enum rev_info_stdin stdin_handling;This was changed to an enum, because (looking at the next patches) we want to add an entry to it. Maybe mention it here - at the very least, this will help reviewers check that the addition of extra entries to the enum in future commits will not negatively affect functionality introduced in this commit.
Show 7 quoted lines
> + /* > + * 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_per_option;
The usage of "per" here seems correct to me, but it's not the first meaning that springs to mind. Could this just be called "consumed_stdin"?