From 3c7406d4b595c9e8c8dfcff2739bab9412add5a3 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 26 Jun 2010 14:23:02 -0500 Subject: t7006 (pager): introduce helper for parameterized tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current tests test pager configuration for ‘git log’, but other commands use a different setup procedure and should therefore be tested separately. Add a helper to make this easier. This patch introduces the helper and changes some existing tests to use it. The only functional change should be the introduction of ‘git log - ’ to a few test descriptions. Signed-off-by: Jonathan Nieder Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index 9a83241..b117ebb 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -172,58 +172,94 @@ then test_set_prereq SIMPLEPAGER fi -test_expect_success SIMPLEPAGER 'default pager is used by default' ' +# Use this helper to make it easy for the caller of your +# terminal-using function to specify whether it should fail. +# If you write +# +# your_test() { +# parse_args "$@" +# +# $test_expectation "$cmd - behaves well" " +# ... +# $full_command && +# ... +# " +# } +# +# then your test can be used like this: +# +# your_test expect_(success|failure) [test_must_fail] 'git foo' +# +parse_args() { + test_expectation="test_$1" + shift + if test "$1" = test_must_fail + then + full_command="test_must_fail test_terminal " + shift + else + full_command="test_terminal " + fi + cmd=$1 + full_command="$full_command $1" +} + +parse_args expect_success 'git log' +$test_expectation SIMPLEPAGER "$cmd - default pager is used by default" " unset PAGER GIT_PAGER; test_might_fail git config --unset core.pager && rm -f default_pager_used || cleanup_fail && - cat >$less <<-\EOF && + cat >\$less <<-\EOF && #!/bin/sh wc >default_pager_used EOF - chmod +x $less && + chmod +x \$less && ( - PATH=.:$PATH && + PATH=.:\$PATH && export PATH && - test_terminal git log + $full_command ) && test -e default_pager_used -' +" -test_expect_success TTY 'PAGER overrides default pager' ' +parse_args expect_success 'git log' +$test_expectation TTY "$cmd - PAGER overrides default pager" " unset GIT_PAGER; test_might_fail git config --unset core.pager && rm -f PAGER_used || cleanup_fail && - PAGER="wc >PAGER_used" && + PAGER='wc >PAGER_used' && export PAGER && - test_terminal git log && + $full_command && test -e PAGER_used -' +" -test_expect_success TTY 'core.pager overrides PAGER' ' +parse_args expect_success 'git log' +$test_expectation TTY "$cmd - core.pager overrides PAGER" " unset GIT_PAGER; rm -f core.pager_used || cleanup_fail && PAGER=wc && export PAGER && - git config core.pager "wc >core.pager_used" && - test_terminal git log && + git config core.pager 'wc >core.pager_used' && + $full_command && test -e core.pager_used -' +" -test_expect_success TTY 'GIT_PAGER overrides core.pager' ' +parse_args expect_success 'git log' +$test_expectation TTY "$cmd - GIT_PAGER overrides core.pager" " rm -f GIT_PAGER_used || cleanup_fail && git config core.pager wc && - GIT_PAGER="wc >GIT_PAGER_used" && + GIT_PAGER='wc >GIT_PAGER_used' && export GIT_PAGER && - test_terminal git log && + $full_command && test -e GIT_PAGER_used -' +" test_done -- cgit v0.10.2-6-g49f6 From 8f81449e885ab2b9bac09b5b835314d26f107b3f Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 26 Jun 2010 14:24:50 -0500 Subject: t7006: test pager configuration for several git commands Test choice of pager at several stages of repository setup. This provides some (admittedly uninteresting) examples to keep in mind when considering changes to the setup procedure. Improved-by: Johannes Sixt Signed-off-by: Jonathan Nieder Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index b117ebb..4420f91 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -204,62 +204,94 @@ parse_args() { full_command="$full_command $1" } -parse_args expect_success 'git log' -$test_expectation SIMPLEPAGER "$cmd - default pager is used by default" " - unset PAGER GIT_PAGER; - test_might_fail git config --unset core.pager && - rm -f default_pager_used || - cleanup_fail && +test_default_pager() { + parse_args "$@" + + $test_expectation SIMPLEPAGER "$cmd - default pager is used by default" " + unset PAGER GIT_PAGER; + test_might_fail git config --unset core.pager && + rm -f default_pager_used || + cleanup_fail && + + cat >\$less <<-\EOF && + #!/bin/sh + wc >default_pager_used + EOF + chmod +x \$less && + ( + PATH=.:\$PATH && + export PATH && + $full_command + ) && + test -e default_pager_used + " +} - cat >\$less <<-\EOF && - #!/bin/sh - wc >default_pager_used - EOF - chmod +x \$less && - ( - PATH=.:\$PATH && - export PATH && - $full_command - ) && - test -e default_pager_used -" +test_PAGER_overrides() { + parse_args "$@" -parse_args expect_success 'git log' -$test_expectation TTY "$cmd - PAGER overrides default pager" " - unset GIT_PAGER; - test_might_fail git config --unset core.pager && - rm -f PAGER_used || - cleanup_fail && + $test_expectation TTY "$cmd - PAGER overrides default pager" " + unset GIT_PAGER; + test_might_fail git config --unset core.pager && + rm -f PAGER_used || + cleanup_fail && - PAGER='wc >PAGER_used' && - export PAGER && - $full_command && - test -e PAGER_used -" - -parse_args expect_success 'git log' -$test_expectation TTY "$cmd - core.pager overrides PAGER" " - unset GIT_PAGER; - rm -f core.pager_used || - cleanup_fail && + PAGER='wc >PAGER_used' && + export PAGER && + $full_command && + test -e PAGER_used + " +} - PAGER=wc && - export PAGER && - git config core.pager 'wc >core.pager_used' && - $full_command && - test -e core.pager_used -" - -parse_args expect_success 'git log' -$test_expectation TTY "$cmd - GIT_PAGER overrides core.pager" " - rm -f GIT_PAGER_used || - cleanup_fail && +test_core_pager_overrides() { + parse_args "$@" + + $test_expectation TTY "$cmd - core.pager overrides PAGER" " + unset GIT_PAGER; + rm -f core.pager_used || + cleanup_fail && + + PAGER=wc && + export PAGER && + git config core.pager 'wc >core.pager_used' && + $full_command && + test -e core.pager_used + " +} + +test_GIT_PAGER_overrides() { + parse_args "$@" + + $test_expectation TTY "$cmd - GIT_PAGER overrides core.pager" " + rm -f GIT_PAGER_used || + cleanup_fail && + + git config core.pager wc && + GIT_PAGER='wc >GIT_PAGER_used' && + export GIT_PAGER && + $full_command && + test -e GIT_PAGER_used + " +} - git config core.pager wc && - 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' +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_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_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_GIT_PAGER_overrides expect_success test_must_fail 'git -p nonsense' test_done -- cgit v0.10.2-6-g49f6 From bce2c9ae9ff458b6090953ab9f639255f757a104 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 26 Jun 2010 14:25:37 -0500 Subject: tests: local config file should be honored from subdirs of toplevel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. If --paginate is handled before searching for the git dir, this configuration will be missed. In other words, with --paginate and only with --paginate, any repository-local core.pager setting is being ignored [*]. [*] unless the git directory is ./.git or GIT_DIR or GIT_CONFIG was set explicitly. Add a test to demonstrate this counterintuitive behavior. Noticed while reading over a patch by Duy that fixes it. Cc: Nguyễn Thái Ngọc Duy Improved-by: Johannes Sixt Signed-off-by: Jonathan Nieder Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index 4420f91..2b106be 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -259,6 +259,28 @@ test_core_pager_overrides() { " } +test_core_pager_subdir() { + parse_args "$@" + + $test_expectation TTY "$cmd - core.pager from subdirectory" " + unset GIT_PAGER; + rm -f core.pager_used && + rm -fr sub || + cleanup_fail && + + PAGER=wc && + stampname=\$(pwd)/core.pager_used && + export PAGER stampname && + git config core.pager 'wc >\"\$stampname\"' && + mkdir sub && + ( + cd sub && + $full_command + ) && + test -e core.pager_used + " +} + test_GIT_PAGER_overrides() { parse_args "$@" @@ -277,21 +299,25 @@ test_GIT_PAGER_overrides() { 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_failure '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_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_done -- cgit v0.10.2-6-g49f6 From 73e25e7cc80e175b5c94678188333e42107fa16e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Sat, 26 Jun 2010 14:26:37 -0500 Subject: git --paginate: do not commit pager choice too early MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Jonathan Nieder Acked-by: Jeff King Signed-off-by: Junio C Hamano 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 -- cgit v0.10.2-6-g49f6 From 030149a4dcd584f6b47221f5b087d081e582d790 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Wed, 14 Jul 2010 17:55:12 -0500 Subject: git --paginate: paginate external commands again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano 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 -- cgit v0.10.2-6-g49f6