git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Jun 26, 2010, 19:26 UTC
Message-ID
<20100626192637.GD20051@burratino>
In-Reply-To
<20100626192203.GA19973@burratino>
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

When git is passed the --paginate option, starting up a pager requires deciding what pager to start, which requires access to the core.pager configuration.

At the relevant moment, the repository has not been searched for yet. Attempting to access the configuration at this point results in git_dir being set to .git [*], which is almost certainly not what was wanted. In particular, when run from a subdirectory of the toplevel, git --paginate does not respect the core.pager setting from the current repository.

[*] unless GIT_DIR or GIT_CONFIG is set
So delay the pager startup when possible:
1. run_argv() already commits pager choice inside run_builtin() if a
   command is found.  For commands that use RUN_SETUP, waiting until
   then fixes the problem described above: once git knows where to
   look, it happily respects the core.pager setting.
2. list_common_cmds_help() prints out 29 lines and exits.  This can
   benefit from pagination, so we need to commit the pager choice
   before writing this output.
   Luckily ‘git’ without subcommand has no other reason to access a
   repository, so it would be intuitive to ignore repository-local
   configuration in this case.  Simpler for now to choose a pager
   using the funny code that notices a repository that happens to be
   at .git.  That this accesses a repository when it is very
   convenient to is a bug but not an important one.
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>
---
Thanks for reading.
 git.c            |    2 +-
 t/t7006-pager.sh |   58 ++++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 47 insertions(+), 13 deletions(-)
diff --git a/git.c b/git.c
index 99f0363..25e7693 100644
--- a/git.c
+++ b/git.c
@@ -511,12 +511,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 2b106be..eefef45 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -244,9 +244,21 @@ test_PAGER_overrides() {
 }
 
 test_core_pager_overrides() {
+	if_local_config=
+	used_if_wanted='overrides PAGER'
+	test_core_pager "$@"
+}
+
+test_local_config_ignored() {
+	if_local_config='! '
+	used_if_wanted='is not used'
+	test_core_pager "$@"
+}
+
+test_core_pager() {
 	parse_args "$@"
 
-	$test_expectation TTY "$cmd - core.pager overrides PAGER" "
+	$test_expectation TTY "$cmd - repository-local core.pager setting $used_if_wanted" "
 		unset GIT_PAGER;
 		rm -f core.pager_used ||
 		cleanup_fail &&
@@ -255,14 +267,26 @@ test_core_pager_overrides() {
 		export PAGER &&
 		git config core.pager 'wc >core.pager_used' &&
 		$full_command &&
-		test -e core.pager_used
+		${if_local_config}test -e core.pager_used
 	"
 }
 
 test_core_pager_subdir() {
+	if_local_config=
+	used_if_wanted='overrides PAGER'
+	test_pager_subdir_helper "$@"
+}
+
+test_no_local_config_subdir() {
+	if_local_config='! '
+	used_if_wanted='is not used'
+	test_pager_subdir_helper "$@"
+}
+
+test_pager_subdir_helper() {
 	parse_args "$@"
 
-	$test_expectation TTY "$cmd - core.pager from subdirectory" "
+	$test_expectation TTY "$cmd - core.pager $used_if_wanted from subdirectory" "
 		unset GIT_PAGER;
 		rm -f core.pager_used &&
 		rm -fr sub ||
@@ -277,7 +301,7 @@ test_core_pager_subdir() {
 			cd sub &&
 			$full_command
 		) &&
-		test -e core.pager_used
+		${if_local_config}test -e core.pager_used
 	"
 }
 
@@ -296,6 +320,20 @@ test_GIT_PAGER_overrides() {
 	"
 }
 
+test_doesnt_paginate() {
+	parse_args "$@"
+
+	$test_expectation TTY "no pager for '$cmd'" "
+		rm -f GIT_PAGER_used ||
+		cleanup_fail &&
+
+		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'
@@ -305,19 +343,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.1.579.ge2549
Previous: Jonathan NiederNext: Jeff King
Message 6 of 42 in “git --paginate: do not commit pager choice too early”
  1. 0/4 git --paginate: do not commit pager choice too earlyJonathan Nieder, Jun 26, 2010
  2. 1/4 t7006 (pager): introduce helper for parameterized testsJonathan Nieder, Jun 26, 2010
  3. 2/4 t7006: test pager configuration for several git commandsJonathan Nieder, Jun 26, 2010
  4. Jonathan NiederJun 26, 2010
  5. 3/4 tests: local config file should be honored from subdirs of toplevelJonathan Nieder, Jun 26, 2010
  6. 4/4 git --paginate: do not commit pager choice too earlyJonathan Nieder, Jun 26, 2010
  7. Jeff KingJun 28, 2010
  8. Jonathan NiederJun 28, 2010
  9. Jeff KingJun 28, 2010
  10. Nguyen Thai Ngoc DuyJun 28, 2010
  11. Junio C HamanoJun 29, 2010
  12. Junio C HamanoJul 14, 2010
  13. Jonathan NiederJul 14, 2010
  14. git --paginate: paginate external commands againJonathan Nieder, Jul 14, 2010
  15. Nguyen Thai Ngoc DuyJul 18, 2010
  16. 0/12 Re: git --paginate: do not commit pager choice too earlyJonathan Nieder, Aug 6, 2010
  17. 01/12 git wrapper: introduce startup_info structJonathan Nieder, Aug 6, 2010
  18. 02/12 setup: remember whether repository was foundJonathan Nieder, Aug 6, 2010
  19. 03/12 git wrapper: allow setup_git_directory_gently() be called earlierJonathan Nieder, Aug 6, 2010
  20. 04/12 shortlog: run setup_git_directory_gently() soonerJonathan Nieder, Aug 6, 2010
  21. 05/12 grep: run setup_git_directory_gently() soonerJonathan Nieder, Aug 6, 2010
  22. 06/12 apply: run setup_git_directory_gently() soonerJonathan Nieder, Aug 6, 2010
  23. Ævar Arnfjörð BjarmasonAug 15, 2010
  24. Nguyen Thai Ngoc DuyAug 15, 2010
  25. Jonathan NiederAug 15, 2010
  26. 06/12 apply: run setup_git_directory_gently() soonerNguyễn Thái Ngọc Duy, Aug 16, 2010
  27. Junio C HamanoAug 16, 2010
  28. 07/12 bundle: run setup_git_directory_gently() soonerJonathan Nieder, Aug 6, 2010
  29. Thomas RastAug 16, 2010
  30. Jonathan NiederAug 16, 2010
  31. 2/2 t7006 (pager): add missing TTY prerequisiteJonathan Nieder, Aug 16, 2010
  32. Junio C HamanoAug 16, 2010
  33. 08/12 config: run setup_git_directory_gently() soonerJonathan Nieder, Aug 6, 2010
  34. 09/12 index-pack: run setup_git_directory_gently() soonerJonathan Nieder, Aug 6, 2010
  35. 10/12 ls-remote: run setup_git_directory_gently() soonerJonathan Nieder, Aug 6, 2010
  36. 11/12 var: run setup_git_directory_gently() soonerJonathan Nieder, Aug 6, 2010
  37. 12/12 merge-file: run setup_git_directory_gently() soonerJonathan Nieder, Aug 6, 2010
  38. 0/2 fix "check-ref-format --branch" from subdir of toplevelJonathan Nieder, Aug 6, 2010
  39. 1/2 check-ref-format: handle subcommands in separate functionsJonathan Nieder, Aug 6, 2010
  40. 2/2 Allow "check-ref-format --branch" from subdirectoryJonathan Nieder, Aug 6, 2010
  41. Junio C HamanoAug 6, 2010
  42. Nguyen Thai Ngoc DuyAug 6, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.