Re: [PATCH] checkout: add --progress option
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Oct 30, 2015, 19:04 UTC
- Message-ID
- <CAPig+cSw7+-vfsWtfP2y4eNijuWdauWe-HfgbOvUt1vYFUEjAg@mail.gmail.com>
- In-Reply-To
- <xmqqh9l8kxa3.fsf@gitster.mtv.corp.google.com>
On Fri, Oct 30, 2015 at 2:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 37 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes:
>> On Thu, Oct 29, 2015 at 9:23 PM, Edmundo Carmona Antoranz
>> <eantoranz@gmail.com> wrote:
>>> + opts.verbose_update = !o->quiet && (option_progress > 0 ||
>>> + (option_progress < 0 && isatty(2)));
>>
>> Does this logic also need to be applied to the other instance where
>> isatty() is consulted in merge_working_tree()?
>
> This makes me wonder if option_progress and o->quiet change once
> parse_options() finished doing its job. If we know that these two
> will never change once parse_options() is done, then we can
> introduce a variable "show_progress", assign the value of that
> variable to opts.verbose_update in these places and then arrange
> that variable is set to the combination of quiet, option_progress
> and isatty(2) just once after parse_options().
>
> That is, something like this on top of Edmundo's patch.
>
> @@ -27,7 +27,7 @@ static const char * const checkout_usage[] = {
> -static int option_progress = -1;
> +static int show_progress = 1;
> @@ -430,8 +430,7 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
> - opts.verbose_update = !o->quiet && (option_progress > 0 ||
> - (option_progress < 0 && isatty(2)));
> + opts.verbose_update = show_progress;
> @@ -515,7 +514,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
> - topts.verbose_update = !opts->quiet && isatty(2);
> + topts.verbose_update = show_progress;
> @@ -1143,6 +1142,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
> + int option_progress = -1;
> @@ -1187,6 +1187,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
> argc = parse_options(argc, argv, prefix, options, checkout_usage,
> PARSE_OPT_KEEP_DASHDASH);
>
> + show_progress = (!opts.quiet &&
> + (0 < option_progress || (option_progress < 0 && isatty(2))));Yep, I was thinking that too. Also, Edmundo's explanatory comment about how --quiet, --progress, and isatty() interact would move down to this location (if it's deemed worth keeping).