Re: [PATCH 0/4] git --paginate: do not commit pager choice too early

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 14, 2010, 21:30 UTC
Message-ID: <20100714213043.GA1590@burratino>
In-Reply-To: <7v630hyf5r.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

> ... and for commands that do not use RUN_SETUP, what happens?

run_builtin() still commits pager choice. The bug is neither fixed nor made worse for them.

> Missing from the above enumeration are are external commands.  They depend
> on commit_pager_choice() to be called before execv_dashed_external() gets
> called.  For example, "git -p request-pull $args" no longer works with
> this patch.

Something like the following may help.

This does not protect against calling setup_pager() more than once. Once the first pager has been set up, isatty(1) is false, preventing additional pagers from being spawned and competing with it.

As futureproofing against --paginate=always, setup_pager() should probably be taught to check pager_in_use(), but that should probably wait for a separate patch.

diff --git a/git.c b/git.c
index 25e7693..2dcd952 100644
--- a/git.c
+++ b/git.c
@@ -162,6 +162,7 @@ static int handle_alias(int *argcp, const char ***argv)
 	int unused_nongit;
 
 	subdir = setup_git_directory_gently(&unused_nongit);
+	commit_pager_choice();
 
 	alias_command = (*argv)[0];
 	alias_string = alias_lookup(alias_command);
@@ -433,6 +434,7 @@ static void execv_dashed_external(const char **argv)
 	int status;
 
 	strbuf_addf(&cmd, "git-%s", argv[0]);
+	commit_pager_choice();
 
 	/*
 	 * argv[0] must be the git command, but the argv array