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

From

Jonathan Nieder <jrnieder@gmail.com>

Date

Apr 13, 2010, 02:24 UTC

Message-ID

<20100413022420.GC4118@progeny.tock>

In-Reply-To

<20100413021153.GA3978@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 to 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>
---
Duy’s version did not allow paginating list_common_cmds_help() output.
I have an idea for fixing the setup procedure for that later, but one
thing at a time.  In other words, this is analogous to a lock pushdown
in a way: the immediate goal is not to eliminate problems but to reduce
their scope.

 git.c            |    2 +-
 t/t7006-pager.sh |   65 +++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 46 insertions(+), 21 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 403c260..944e830 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -132,23 +132,28 @@ then
 fi
 
 test_pager_choice() {
-	if test $# = 1
+	wrapper=test_terminal
+
+	if test "$1" = test_must_fail
 	then
-		cmd=$1
-		full_cmd="test_terminal $cmd"
-	else
-		cmd=$2
-		full_cmd="test_must_fail test_terminal $cmd"
+		wrapper="test_must_fail $wrapper"
+		shift
 	fi
 
-	case "$cmd" in
-	*-p*)
-		test_expect_expected=test_expect_failure
-		;;
-	*)
-		test_expect_expected=test_expect_success
-		;;
-	esac
+	cmd=$1
+	toplevel_expectation=${2:-success}
+	use_repository_config="${3:-yes}"
+
+	full_cmd="$wrapper $cmd"
+	test_expectation="test_expect_$toplevel_expectation"
+	if test "$use_repository_config" = no
+	then
+		if_want_local_config='! '
+		used_if_wanted='is not used'
+	else
+		if_want_local_config=
+		used_if_wanted='overrides PAGER'
+	fi
 
 	unset PAGER GIT_PAGER
 	git config --unset core.pager
@@ -175,18 +180,18 @@ test_pager_choice() {
 
 	unset GIT_PAGER
 	rm -f core.pager_used
-	test_expect_success 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_cmd &&
-		test -e core.pager_used
+		${if_want_local_config}test -e core.pager_used
 	"
 
 	unset GIT_PAGER
 	rm -f core.pager_used
 	rm -fr sub
-	$test_expect_expected TTY "$cmd - core.pager in subdir" "
+	test_expect_success TTY "$cmd - core.pager ${used_if_wanted} from subdirectory" "
 		PAGER=wc &&
 		stampname=\$(pwd)/core.pager_used &&
 		export PAGER stampname &&
@@ -196,7 +201,7 @@ test_pager_choice() {
 			cd sub &&
 			$full_cmd
 		) &&
-		test -e \"\$stampname\"
+		${if_want_local_config}test -e \"\$stampname\"
 	"
 
 	rm -f GIT_PAGER_used
@@ -209,9 +214,29 @@ test_pager_choice() {
 	"
 }
 
+doesnt_paginate() {
+	if test $# = 1
+	then
+		cmd=$1
+		full_cmd="test_terminal $cmd"
+	else
+		cmd=$2
+		full_cmd="test_must_fail test_terminal $cmd"
+	fi
+
+	rm -f GIT_PAGER_used
+	test_expect_success TTY "no pager for '$cmd'" "
+		GIT_PAGER='wc > GIT_PAGER_used' &&
+		export GIT_PAGER
+		$full_cmd &&
+		! test -e GIT_PAGER_used
+	"
+}
+
 test_pager_choice 'git log'
 test_pager_choice 'git -p log'
-test_pager_choice test_must_fail 'git -p'
-test_pager_choice test_must_fail 'git -p nonsense'
+test_pager_choice test_must_fail 'git -p' failure no
+
+doesnt_paginate test_must_fail 'git -p nonsense'
 
 test_done

-- 
1.7.0.4