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

[Avery Pennarun <apenwarr@gmail.com>]

On Thu, May 24, 2012 at 2:29 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 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?

> 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