Re: [PATCH] checkout: add --progress option

[Jeff King <peff@peff.net>]

On Sat, Oct 31, 2015 at 12:14:39PM -0600, Edmundo Carmona Antoranz wrote:

> On Sat, Oct 31, 2015 at 11:42 AM, Edmundo Carmona Antoranz
> <eantoranz@gmail.com> wrote:
> > On Sat, Oct 31, 2015 at 11:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
> >> I do find what Peff showed us a lot easier to follow.
> >>
> >>         if (opts.show_progress < 0) {
> >>                 if (opts.quiet)
> >>                         opts.show_progress = 0;
> >>                 else
> >>                         opts.show_progress = isatty(2);
> >>         }
> >>
> >
> > Ok.... let me rewrite it that way. Other than that, the other things are ok?
> 
> In Peff's implementation I think he uses -1 as --no-progress, 1 as
> --progress and 0 as undefined, right?

I didn't mean to, though I don't promise I didn't send something buggy.
It looks right to me, though:

  if (opts.show_progress < 0) { /* if the user didn't say... */
          if (opts.quiet)
		opts.show_progress = 0; /* quiet means "no progress" */
	  else
		opts.show_progress = isatty(2); /* returns 0/1 bool */
  }

> In my implementation I'm using -1 as undefined and 0 as --no-progress.
> What would be the standard approach?

That's standard. And "1" is "--progress".

> From what I can see on
> parse_options's behavior, if you select --no-progress, the variable
> ends up with a 0, which makes me think I'm using the right approach.
> 
> End result with my assumptions would be:
> 
>         if (opts.show_progress) {
>                 /* user selected --progress or didn't specify */
>                 if (opts.quiet) {
>                         opts.show_progress = 0;
>                 } else if (opts.show_progress < 0) {
>                         opts.show_progress = isatty(2);
>                 }
>         }

The difference between mine and yours is that in mine, "--progress"
trumps "--quiet", whereas it is the other way around in yours. I don't
know if it is a huge deal, but mine makes more sense to me (because
"--progress" is more specific than "--quiet", which might silence other
messages, too).

-Peff