[PATCH] git --paginate: paginate external commands again

[Jonathan Nieder <jrnieder@gmail.com>]

73e25e7c (git --paginate: do not commit pager choice too early,
2010-06-26) failed to take some cases into account.

1b. Builtins that do not use RUN_SETUP (like git config) do
    not find GIT_DIR set correctly when the pager is launched
    from run_builtin().  So the core.pager configuration is
    not honored from subdirectories of the toplevel for them.

4a. External git commands (like git request-pull) relied on the
    early pager launch to take care of handling the -p option.
    Ever since 73e25e7c, they do not honor the -p option at all.

4b. Commands invoked through ! aliases (like ls) were also relying
    on the early pager launch.

Fix (4a) by launching the pager (if requested) before running such a
“dashed external”.  For simplicity, this still does not search for a
.git directory before running the external command; when run from a
subdirectory of the toplevel, therefore, the “[core] pager”
configuration is still not honored.

Fix (4b) by launching pager if requested before carrying out such an
alias.  Actually doing this has no effect, since the pager (if any)
would have already been launched in a failed attempt to try a
dashed external first.  The choice-of-pager-not-honored-from-
subdirectory bug still applies here, too.

(1b) is not a regression.  There is no need to fix it yet.

Noticed by Junio.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:

> I hate an enumeration that pretends to be exhausitive but is not.

Sorry about that.

Here’s a patch that could be squashed in or applied on top.

 git.c            |    3 +++
 t/t7006-pager.sh |   47 +++++++++++++++++++++++++++++++++++------------
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/git.c b/git.c
index 25e7693..99517dd 100644
--- a/git.c
+++ b/git.c
@@ -167,6 +167,7 @@ static int handle_alias(int *argcp, const char ***argv)
 	alias_string = alias_lookup(alias_command);
 	if (alias_string) {
 		if (alias_string[0] == '!') {
+			commit_pager_choice();
 			if (*argcp > 1) {
 				struct strbuf buf;
 
@@ -432,6 +433,8 @@ static void execv_dashed_external(const char **argv)
 	const char *tmp;
 	int status;
 
+	commit_pager_choice();
+
 	strbuf_addf(&cmd, "git-%s", argv[0]);
 
 	/*
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index eefef45..9215c2f 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -334,17 +334,40 @@ test_doesnt_paginate() {
 	"
 }
 
-test_default_pager        expect_success 'git log'
-test_PAGER_overrides      expect_success 'git log'
-test_core_pager_overrides expect_success 'git log'
-test_core_pager_subdir    expect_success 'git log'
-test_GIT_PAGER_overrides  expect_success 'git log'
-
-test_default_pager        expect_success 'git -p log'
-test_PAGER_overrides      expect_success 'git -p log'
-test_core_pager_overrides expect_success 'git -p log'
-test_core_pager_subdir    expect_success 'git -p log'
-test_GIT_PAGER_overrides  expect_success 'git -p log'
+test_pager_choices() {
+	test_default_pager        expect_success "$@"
+	test_PAGER_overrides      expect_success "$@"
+	test_core_pager_overrides expect_success "$@"
+	test_core_pager_subdir    expect_success "$@"
+	test_GIT_PAGER_overrides  expect_success "$@"
+}
+
+test_expect_success 'setup: some aliases' '
+	git config alias.aliasedlog log &&
+	git config alias.true "!true"
+'
+
+test_pager_choices                       'git log'
+test_pager_choices                       'git -p log'
+test_pager_choices                       'git aliasedlog'
+
+test_default_pager        expect_success 'git -p aliasedlog'
+test_PAGER_overrides      expect_success 'git -p aliasedlog'
+test_core_pager_overrides expect_success 'git -p aliasedlog'
+test_core_pager_subdir    expect_failure 'git -p aliasedlog'
+test_GIT_PAGER_overrides  expect_success 'git -p aliasedlog'
+
+test_default_pager        expect_success 'git -p true'
+test_PAGER_overrides      expect_success 'git -p true'
+test_core_pager_overrides expect_success 'git -p true'
+test_core_pager_subdir    expect_failure 'git -p true'
+test_GIT_PAGER_overrides  expect_success 'git -p true'
+
+test_default_pager        expect_success test_must_fail 'git -p request-pull'
+test_PAGER_overrides      expect_success test_must_fail 'git -p request-pull'
+test_core_pager_overrides expect_success test_must_fail 'git -p request-pull'
+test_core_pager_subdir    expect_failure test_must_fail 'git -p request-pull'
+test_GIT_PAGER_overrides  expect_success test_must_fail 'git -p request-pull'
 
 test_default_pager        expect_success test_must_fail 'git -p'
 test_PAGER_overrides      expect_success test_must_fail 'git -p'
@@ -352,6 +375,6 @@ test_local_config_ignored expect_failure test_must_fail 'git -p'
 test_no_local_config_subdir expect_success test_must_fail 'git -p'
 test_GIT_PAGER_overrides  expect_success test_must_fail 'git -p'
 
-test_doesnt_paginate      expect_success test_must_fail 'git -p nonsense'
+test_doesnt_paginate      expect_failure test_must_fail 'git -p nonsense'
 
 test_done
-- 
1.7.2.rc2.532.g65d9b.dirty