Re: [PATCH v2] checkout: no progress messages if !isatty(2).

From: Avery Pennarun <apenwarr@gmail.com>
Date: May 24, 2012, 18:46 UTC
Message-ID: <CAHqTa-3QUsW_AP67NWjc-Gu5FZ7xQZyOOM-=zea+vwZeT79=0A@mail.gmail.com>
In-Reply-To: <7vy5ohwhy7.fsf@alter.siamese.dyndns.org>

On Thu, May 24, 2012 at 2:29 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 12 quoted lines

> Avery Pennarun <apenwarr@gmail.com> writes:
>> It would probably be better to have progress.c check isatty(2) all the time,
>> but that wouldn't allow things like 'git push --progress' to force progress
>> reporting to on, so I won't try to solve the general case right now.
>
> Before that "It would probably be better" comment to give your opinion,
> you need to describe what problem you wanted to solve in the first place.
> I'll lift it from your original version of the patch:
>
>    If stderr isn't a tty, we shouldn't be printing incremental progress
>    messages.  In particular, this affected 'git checkout -f . >&logfile'
>    unless you provided -q.  And git-new-workdir has no way to provide -q.

Do you want me to rephrase the commit message and resend?

Show 7 quoted lines

> I do not seem to find a sane justification for
>
>        git $cmd --progress 2>output
>
> use case and I do not immediately see how that "output" file can be
> useful.  But we've allowed it for a long time, so probably this version is
> safer.  Besides, it is more explicit.

Yeah, I have nothing against allowing --progress to work. If I were to clarify my comment above, it would be to say that I'm worried about how *many* places we keep calling isatty(). It is (as we can see from the need for this patch) error prone, since I think most naive coders would expect the progress stuff to act correctly by default if !isatty(2).

So maybe the "right" fix is to add a flag to start_progress_delay() to "force" verbose mode; if it's not set, start_progress_delay() would check isatty(2) and decide automatically what to do. This wouldn't save much code, but would make sure developers think about their intentions.

Have fun,

Avery