On Fri, May 02, 2008 at 11:18:02AM -0700, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> writes:
>
> > My bigger worry is that this affects only builtins. Which makes it
> > sufficient for turning off the pager for anything that does USE_PAGER.
>
> Hmm. How about doing things this way?
>
> - at the beginning of handle_options() remember argv[0]
>
> - restructure handle_options() so that it does not run setup_pager() and
> setenv("GIT_PAGER", "cat", 1) inside the loop, but instead remember
> what we had on the command line;
>
> - after the handle_options() loop, if we saw an explicit --pager,
> --no-pager, that's the decision;
>
> - otherwise:
>
> - look at argv[0] to see what the command is;
>
> - do the config thing to see if there is user preference; if there is
> one, that setting decides;
>
> - otherwise:
>
> - see the built-in defaults;
>
> - and finally use or not use pager depending on what we found above.
OK, that makes some sense. I think some of what you describe is just
refactoring (e.g., it doesn't matter if we actually do things when we
see --no-pager or afterwards, since it always takes precedence). The key
things are:
- work not just on running builtins, but before we even figure out
whether we have a builtin or a script
- in my patch the config just says "ignore the default USE_PAGER", but
it really should be "turn off the pager via GIT_PAGER=cat". That way
you can say pager.stash = false, and it will impact the git-diff
invocation run by stash.
But that isn't to say the refactoring isn't worth doing to keep things
clean. I will take a stab at restructuring it the way you specified.
There is one remaining annoyance, though: this code is only run via the
git wrapper. That means that you will get different behavior for
"git-stash" versus "git stash". To make that work, we would have to put
equivalent support into each script (although we could hit several at
once with git-sh-setup.sh) and each non-builtin.
-Peff
--
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
