Re: [PATCH] pager: do not leak "GIT_PAGER_IN_USE" to the pager

[Jeff King <peff@peff.net>]

On Fri, Jul 03, 2015 at 10:18:45AM -0700, Junio C Hamano wrote:

> Since 2e6c01e2, we export GIT_PAGER_IN_USE so that a process that

I imagine you mean 2e6c012e here.

> becomes the upstream of the spawned pager can still tell that we
> have spawned the pager and decide to do colored output even when
> its output no longer goes to a terminal (i.e. isatty(1)).
> 
> But we forgot to clear it from the enviornment of the spawned pager.
> This is not a problem in a sane world, but if you have a handful of
> thousands Git users in your organization, somebody is bound to do
> strange things, e.g. typing "!<ENTER>" instead of 'q' to get control
> back from $LESS.  GIT_PAGER_IN_USE is still set in that subshell
> spawned by "less", and all sorts of interesting things starts
> happening, e.g. "git diff | cat" starts coloring its output.
> 
> We can clear the environment variable in the half of the fork that
> runs the pager to avoid the confusion.

I think this is a reasonable fix. I guess it would be possible that some
pager would want to react differently depending on the variable, but I
could not think of a useful case. And certainly your pager, being the
pager itself, can assume that the pager is in use. ;) At the very worst,
somebody can set GIT_PAGER="GIT_PAGER_IN_USE=1 my-pager" if they truly
want to do something bizarre.

So,

Acked-by: Jeff King <peff@peff.net>

-Peff