git --paginate: do not commit pager choice too early

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: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/git.c b/git.c
index 99f036302a7e6d884369d1d3f4ce428e437cbccd..25e76939e2b9b3e89f741cde128cbc447d8f3dbb 100644 (file)
--- 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 2b106be9e9da60279b21036306472f3884e19487..eefef45cc39fc91e44e9645569b8f08828ea2652 100755 (executable)
--- 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