Re: [PATCH 0/7] [RFC] advice: refuse to output if stderr not TTY

From: Jeff King <peff@peff.net>
Date: Aug 21, 2024, 15:40 UTC
Message-ID: <20240821154001.GA506216@coredump.intra.peff.net>
In-Reply-To: <pull.1776.git.1724238152.gitgitgadget@gmail.com>

On Wed, Aug 21, 2024 at 11:02:25AM +0000, Derrick Stolee via GitGitGadget wrote:

Show 8 quoted lines

> Advice is supposed to be for humans, not machines. Why do we output it when
> stderr is not a terminal? Let's stop doing that.
> 
> I'm labeling this as an RFC because I believe there is some risk with this
> change. In particular, this does change behavior to reduce the output that
> some scripts may depend upon. But this output is not intended to be locked
> in and we add or edit advice messages without considering this impact, so
> there is risk in the existing system already.

Playing devil's advocate for a moment: what about programs that read stderr but intend to relay the output to the user?

For example, programs running on the server side of a push are spawned by receive-pack with their stderr fed into a muxer that ships it to the client, who then dumps it to the user's terminal. Would we ever want to see their advice?

My guess is "conceivably yes", though I don't know of a specific example (and in fact, I've seen the "your hook was ignored because it's not executable" advice coming from a server, which was actually more of an annoyance on the client side).

Ditto for upload-pack. Another possible place where it matters: interfaces that wrap Git and collect the output to show to the user. I don't use git-gui, but I'd imagine it does this in some places.

Looking over patch 7, I think the escape hatch for all of these cases would be setting GIT_ADVICE=1. Which isn't too bad, but it does require some action. I'm not sure if it is worth it (but then, I am not all that sympathetic to the script you mentioned that was trying to be too clever about parsing stderr).

-Peff