On Thu, May 24, 2012 at 2:29 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Avery Pennarun <apenwarr@xxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html