Re: To page or not to page

[Jeff King <peff@peff.net>]

On Fri, May 02, 2008 at 11:18:02AM -0700, Junio C Hamano wrote:

> 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;

This is actually a bit tricky, since there are two code paths; builtins
called as git-foo never even go to handle_options, but they can still
look up the config value.

So how about this:

 - keep a global use_pager = { 0 (explicit no), 1 (explicit yes), -1
   (unknown) }

 - if git-foo, lookup config for pager.foo

 - otherwise we have "git [options] foo"; look for -p / --no-pager; if
   none found, then lookup config for "foo"

 - before proceeding further, "commit" the pager choice by running it
   (if 1), munging GIT_PAGER=cat (if 0), or doing nothing (if -1)

 - before handling an internal command, if use_pager is -1 and the
   command defaults to a pager, we run it then

The patch below implements this.

It would be nice to actually defer running the pager until we are about
to run a git command. I.e., never "commit" to the pager until we are
actually running an internal command or exec'ing an external command.
That way it would be safe to make an alias that called "--no-pager"
(which is currently disallowed).

That works for internal commands, since we know we are going to run one,
and if it fails, we die.  However, for external commands, we just exec
and hope it works. So if we run the pager beforehand, we are now
committed to it, and a further alias cannot set --no-pager. We could
alternatively run it after deciding that running the command is what
we'll do, but then we have to do the PATH lookup ourselves.

So anyway, here is the less invasive version.

---
 git.c |   48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/git.c b/git.c
index 89b431f..dadca41 100644
--- a/git.c
+++ b/git.c
@@ -6,12 +6,46 @@
 const char git_usage_string[] =
 	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
 
+static int use_pager = -1;
+static const char *pager_command_key;
+static int pager_command_value;
+
+int pager_command_config(const char *var, const char *value)
+{
+	if (!prefixcmp(var, "pager.") && !strcmp(var + 6, pager_command_key))
+		pager_command_value = git_config_bool(var, value);
+	return 0;
+}
+
+/* returns 0 for "no pager", 1 for "use pager", and -1 for "not specified" */
+int check_pager_config(const char *cmd)
+{
+	pager_command_key = cmd;
+	pager_command_value = -1;
+	git_config(pager_command_config);
+	return pager_command_value;
+}
+
+static void commit_pager_choice(void) {
+	switch (use_pager) {
+	case 0:
+		setenv("GIT_PAGER", "cat", 1);
+		break;
+	case 1:
+		setup_pager();
+		break;
+	default:
+		break;
+	}
+}
+
 static int handle_options(const char*** argv, int* argc, int* envchanged)
 {
 	int handled = 0;
+	const char *cmd = NULL;
 
 	while (*argc > 0) {
-		const char *cmd = (*argv)[0];
+		cmd = (*argv)[0];
 		if (cmd[0] != '-')
 			break;
 
@@ -35,9 +69,9 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
 				exit(0);
 			}
 		} else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) {
-			setup_pager();
+			use_pager = 1;
 		} else if (!strcmp(cmd, "--no-pager")) {
-			setenv("GIT_PAGER", "cat", 1);
+			use_pager = 0;
 			if (envchanged)
 				*envchanged = 1;
 		} else if (!strcmp(cmd, "--git-dir")) {
@@ -84,6 +118,9 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
 		(*argc)--;
 		handled++;
 	}
+
+	if (use_pager == -1 && cmd)
+		use_pager = check_pager_config(cmd);
 	return handled;
 }
 
@@ -239,7 +276,7 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv)
 	prefix = NULL;
 	if (p->option & RUN_SETUP)
 		prefix = setup_git_directory();
-	if (p->option & USE_PAGER)
+	if (use_pager == -1 && p->option & USE_PAGER)
 		setup_pager();
 	if (p->option & NEED_WORK_TREE)
 		setup_work_tree();
@@ -411,6 +448,8 @@ int main(int argc, const char **argv)
 	if (!prefixcmp(cmd, "git-")) {
 		cmd += 4;
 		argv[0] = cmd;
+		use_pager = check_pager_config(cmd);
+		commit_pager_choice();
 		handle_internal_command(argc, argv);
 		die("cannot handle %s internally", cmd);
 	}
@@ -419,6 +458,7 @@ int main(int argc, const char **argv)
 	argv++;
 	argc--;
 	handle_options(&argv, &argc, NULL);
+	commit_pager_choice();
 	if (argc > 0) {
 		if (!prefixcmp(argv[0], "--"))
 			argv[0] += 2;
-- 
1.5.5.1.244.g148a.dirty