[PATCH 3/9 v2] builtins: do not commit pager choice early

From

Jonathan Nieder <jrnieder@gmail.com>

Date

Apr 14, 2010, 02:17 UTC

Message-ID

<20100414021704.GA22766@progeny.tock>

In-Reply-To

<20100413022420.GC4118@progeny.tock>

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

1. list_common_cmds_help() is a printout of 29 lines.  This can
   benefit from pagination, so do commit the pager choice before
   writing this output.

   Since ‘git’ without subcommand has no other reason to access the
   repository, ideally this would not involve accessing the
   repository-specific configuration file.  But for now, it is simpler
   to commit the pager choice using the funny code that would notice
   whatever repository happens to be at .git.  That this accesses a
   repository if it happens to be very convenient is a bug but not
   a very important one.

2. run_argv() already commits pager choice inside run_builtin() if a
   command is found.  If the relevant git subcommand searches for a
   git directory, not commiting to a pager before then means the
   core.pager setting from the correct $GIT_DIR/config will be
   respected.

3. help_unknown_cmd() prints out a few lines to stderr.  It is not
   important to paginate this, so don’t.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Rebased on top of patch 2 v2

 git.c            |    2 +-
 t/t7006-pager.sh |   63 +++++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 52 insertions(+), 13 deletions(-)

diff --git a/git.c b/git.c
index 6bae305..32720e5 100644
--- a/git.c
+++ b/git.c
@@ -502,12 +502,12 @@ 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;
 	} else {
 		/* The user didn't specify a command; give them help */
+		commit_pager_choice();
 		printf("usage: %s\n\n", git_usage_string);
 		list_common_cmds_help();
 		printf("\n%s\n", git_more_info_string);
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index d5f8a18..a240b15 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -190,27 +190,58 @@ test_PAGER_overrides() {
 	"
 }
 
+parse_ifwanted_arg() {
+	if test "$1" = yes
+	then
+		if_want_local_config=
+		used_if_wanted='overrides PAGER'
+	else
+		if_want_local_config='! '
+		used_if_wanted='is not used'
+	fi
+}
+
 test_core_pager_overrides() {
+	test_core_pager yes "$@"
+}
+
+test_local_config_ignored() {
+	test_core_pager no "$@"
+}
+
+test_core_pager() {
+	parse_ifwanted_arg "$1"
+	shift
 	parse_args "$@"
 
 	unset GIT_PAGER
 	rm -f core.pager_used
-	$test_expectation TTY "$cmd - core.pager overrides PAGER" "
+	$test_expectation TTY "$cmd - core.pager $used_if_wanted" "
 		PAGER=wc &&
 		export PAGER &&
 		git config core.pager 'wc > core.pager_used' &&
 		$full_command &&
-		test -e core.pager_used
+		${if_want_local_config}test -e core.pager_used
 	"
 }
 
 test_core_pager_subdir() {
+	test_pager_subdir_helper yes "$@"
+}
+
+test_no_local_config_subdir() {
+	test_pager_subdir_helper no "$@"
+}
+
+test_pager_subdir_helper() {
+	parse_ifwanted_arg "$1"
+	shift
 	parse_args "$@"
 
 	unset GIT_PAGER
 	rm -f core.pager_used
 	rm -fr sub
-	$test_expectation TTY "$cmd - core.pager from subdirectory" "
+	$test_expectation TTY "$cmd - core.pager $used_if_wanted from subdirectory" "
 		stampname=\$(pwd)/core.pager_used &&
 		PAGER=wc &&
 		export PAGER stampname &&
@@ -220,7 +251,7 @@ test_core_pager_subdir() {
 			cd sub &&
 			$full_command
 		) &&
-		test -e \"\$stampname\"
+		${if_want_local_config}test -e \"\$stampname\"
 	"
 }
 
@@ -237,6 +268,18 @@ test_GIT_PAGER_overrides() {
 	"
 }
 
+test_doesnt_paginate() {
+	parse_args "$@"
+
+	rm -f GIT_PAGER_used
+	$test_expectation TTY "no pager for '$cmd'" "
+		GIT_PAGER='wc > GIT_PAGER_used' &&
+		export GIT_PAGER &&
+		$full_command &&
+		! test -e GIT_PAGER_used
+	"
+}
+
 test_default_pager        expect_success 'git log'
 test_PAGER_overrides      expect_success 'git log'
 test_core_pager_overrides expect_success 'git log'
@@ -246,19 +289,15 @@ 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_failure 'git -p log'
+test_core_pager_subdir    expect_success 'git -p log'
 test_GIT_PAGER_overrides  expect_success 'git -p log'
 
 test_default_pager        expect_success test_must_fail 'git -p'
 test_PAGER_overrides      expect_success test_must_fail 'git -p'
-test_core_pager_overrides expect_success test_must_fail 'git -p'
-test_core_pager_subdir    expect_failure test_must_fail 'git -p'
+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_default_pager        expect_success test_must_fail 'git -p nonsense'
-test_PAGER_overrides      expect_success test_must_fail 'git -p nonsense'
-test_core_pager_overrides expect_success test_must_fail 'git -p nonsense'
-test_core_pager_subdir    expect_failure test_must_fail 'git -p nonsense'
-test_GIT_PAGER_overrides  expect_success test_must_fail 'git -p nonsense'
+test_doesnt_paginate expect_success test_must_fail 'git -p nonsense'
 
 test_done

-- 
1.7.0.4