From cd878a206e8ca67b215de5fa510f0ef30bb6e1c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Wed, 21 Feb 2018 19:51:42 +0100 Subject: [PATCH 1/3] t7006: add tests for how git config paginates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The next couple of commits will change how `git config` handles `pager.config`, similar to how de121ffe5 (tag: respect `pager.tag` in list-mode only, 2017-08-02) and ff1e72483 (tag: change default of `pager.tag` to "on", 2017-08-02) changed `git tag`. Similar work has also been done to `git branch`. Add tests in this area to make sure that we don't regress and so that the upcoming commits can be made clearer by adapting the tests. Add tests for simple config-setting, `--edit`, `--get`, `--get-urlmatch`, `get-all`, and `--list`. Those represent a fair portion of the various options that will be affected by the next two commits. Use `test_expect_failure` to document that we currently respect the pager-configuration with `--edit`. The current behavior is buggy since the pager interferes with the editor and makes the end result completely broken. See also b3ee740c8 (t7006: add tests for how git tag paginates, 2017-08-02). The next commit will teach simple config-setting and `--get` to ignore `pager.config`. Test the current behavior as "success", not "failure", since the currently expected behavior according to documentation would be to page. The next commit will change that expectation by updating the documentation on `git config` and will redefine those successful tests. Remove the test added in commit 3ba7e6e29a (config: run setup_git_directory_gently() sooner, 2010-08-05) since it has some overlap with these. We could leave it or tweak it, or place new tests like these next to it, but let's instead make the tests for `git config` as similar as possible to the ones for `git tag` and `git branch`, and place them after those. Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- t/t7006-pager.sh | 49 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index f5f46a95b4..a46a079339 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -110,13 +110,6 @@ test_expect_success TTY 'configuration can disable pager' ' ! test -e paginated.out ' -test_expect_success TTY 'git config uses a pager if configured to' ' - rm -f paginated.out && - test_config pager.config true && - test_terminal git config --list && - test -e paginated.out -' - test_expect_success TTY 'configuration can enable pager (from subdir)' ' rm -f paginated.out && mkdir -p subdir && @@ -252,6 +245,48 @@ test_expect_success TTY 'git branch --set-upstream-to ignores pager.branch' ' ! test -e paginated.out ' +test_expect_success TTY 'git config respects pager.config when setting' ' + rm -f paginated.out && + test_terminal git -c pager.config config foo.bar bar && + test -e paginated.out +' + +test_expect_failure TTY 'git config --edit ignores pager.config' ' + rm -f paginated.out editor.used && + write_script editor <<-\EOF && + touch editor.used + EOF + EDITOR=./editor test_terminal git -c pager.config config --edit && + ! test -e paginated.out && + test -e editor.used +' + +test_expect_success TTY 'git config --get respects pager.config' ' + rm -f paginated.out && + test_terminal git -c pager.config config --get foo.bar && + test -e paginated.out +' + +test_expect_success TTY 'git config --get-urlmatch defaults to not paging' ' + rm -f paginated.out && + test_terminal git -c http."https://foo.com/".bar=foo \ + config --get-urlmatch http https://foo.com && + ! test -e paginated.out +' + +test_expect_success TTY 'git config --get-all respects pager.config' ' + rm -f paginated.out && + test_terminal git -c pager.config config --get-all foo.bar && + test -e paginated.out +' + +test_expect_success TTY 'git config --list defaults to not paging' ' + rm -f paginated.out && + test_terminal git config --list && + ! test -e paginated.out +' + + # A colored commit log will begin with an appropriate ANSI escape # for the first color; the text "commit" comes later. colorful() { From 32888b8fd56175f52f3139915d931f78d8c4ef89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Wed, 21 Feb 2018 19:51:43 +0100 Subject: [PATCH 2/3] config: respect `pager.config` in list/get-mode only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similar to de121ffe5 (tag: respect `pager.tag` in list-mode only, 2017-08-02), use the DELAY_PAGER_CONFIG-mechanism to only respect `pager.config` when we are listing or "get"ing config. We have several getters and some are guaranteed to give at most one line of output. Paging all getters including those could be convenient from a documentation point-of-view. The downside would be that a misconfigured or not so modern pager might wait for user interaction before terminating. Let's instead respect the config for precisely those getters which may produce more than one line of output. `--get-urlmatch` may or may not produce multiple lines of output, depending on the exact usage. Let's not try to recognize the two modes, but instead make `--get-urlmatch` always respect the config. Analyzing the detailed usage might be trivial enough here, but could establish a precedent that we will never be able to enforce throughout the codebase and that will just open a can of worms. This fixes the failing test added in the previous commit. Also adapt the test for whether `git config foo.bar bar` and `git config --get foo.bar` respects `pager.config`. Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- Documentation/git-config.txt | 5 +++++ builtin/config.c | 10 ++++++++++ git.c | 2 +- t/t7006-pager.sh | 10 +++++----- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 14da5fc157..249090ac84 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -233,6 +233,11 @@ See also <>. using `--file`, `--global`, etc) and `on` when searching all config files. +CONFIGURATION +------------- +`pager.config` is only respected when listing configuration, i.e., when +using `--list` or any of the `--get-*` which may return multiple results. + [[FILES]] FILES ----- diff --git a/builtin/config.c b/builtin/config.c index ab5f95476e..a732d9b56c 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -48,6 +48,13 @@ static int show_origin; #define ACTION_GET_COLORBOOL (1<<14) #define ACTION_GET_URLMATCH (1<<15) +/* + * The actions "ACTION_LIST | ACTION_GET_*" which may produce more than + * one line of output and which should therefore be paged. + */ +#define PAGING_ACTIONS (ACTION_LIST | ACTION_GET_ALL | \ + ACTION_GET_REGEXP | ACTION_GET_URLMATCH) + #define TYPE_BOOL (1<<0) #define TYPE_INT (1<<1) #define TYPE_BOOL_OR_INT (1<<2) @@ -594,6 +601,9 @@ int cmd_config(int argc, const char **argv, const char *prefix) usage_with_options(builtin_config_usage, builtin_config_options); } + if (actions & PAGING_ACTIONS) + setup_auto_pager("config", 0); + if (actions == ACTION_LIST) { check_argc(argc, 0, 0); if (config_with_options(show_all_config, NULL, diff --git a/git.c b/git.c index c870b9719c..e5c9b8729d 100644 --- a/git.c +++ b/git.c @@ -389,7 +389,7 @@ static struct cmd_struct commands[] = { { "column", cmd_column, RUN_SETUP_GENTLY }, { "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE }, { "commit-tree", cmd_commit_tree, RUN_SETUP }, - { "config", cmd_config, RUN_SETUP_GENTLY }, + { "config", cmd_config, RUN_SETUP_GENTLY | DELAY_PAGER_CONFIG }, { "count-objects", cmd_count_objects, RUN_SETUP }, { "credential", cmd_credential, RUN_SETUP_GENTLY }, { "describe", cmd_describe, RUN_SETUP }, diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index a46a079339..95bd26f0b2 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -245,13 +245,13 @@ test_expect_success TTY 'git branch --set-upstream-to ignores pager.branch' ' ! test -e paginated.out ' -test_expect_success TTY 'git config respects pager.config when setting' ' +test_expect_success TTY 'git config ignores pager.config when setting' ' rm -f paginated.out && test_terminal git -c pager.config config foo.bar bar && - test -e paginated.out + ! test -e paginated.out ' -test_expect_failure TTY 'git config --edit ignores pager.config' ' +test_expect_success TTY 'git config --edit ignores pager.config' ' rm -f paginated.out editor.used && write_script editor <<-\EOF && touch editor.used @@ -261,10 +261,10 @@ test_expect_failure TTY 'git config --edit ignores pager.config' ' test -e editor.used ' -test_expect_success TTY 'git config --get respects pager.config' ' +test_expect_success TTY 'git config --get ignores pager.config' ' rm -f paginated.out && test_terminal git -c pager.config config --get foo.bar && - test -e paginated.out + ! test -e paginated.out ' test_expect_success TTY 'git config --get-urlmatch defaults to not paging' ' From c0e9f5be873d7c6a5df928f167ec1b27027efb45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Wed, 21 Feb 2018 19:51:44 +0100 Subject: [PATCH 3/3] config: change default of `pager.config` to "on" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is similar to ff1e72483 (tag: change default of `pager.tag` to "on", 2017-08-02) and is safe now that we do not consider `pager.config` at all when we are not listing or getting configuration. This change will help with listing large configurations, but will not hurt users of `git config --edit` as it would have before the previous commit. Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- Documentation/git-config.txt | 1 + builtin/config.c | 2 +- t/t7006-pager.sh | 12 ++++++------ 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 249090ac84..e09ed5d7d5 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -237,6 +237,7 @@ CONFIGURATION ------------- `pager.config` is only respected when listing configuration, i.e., when using `--list` or any of the `--get-*` which may return multiple results. +The default is to use a pager. [[FILES]] FILES diff --git a/builtin/config.c b/builtin/config.c index a732d9b56c..01169dd628 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -602,7 +602,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) } if (actions & PAGING_ACTIONS) - setup_auto_pager("config", 0); + setup_auto_pager("config", 1); if (actions == ACTION_LIST) { check_argc(argc, 0, 0); diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index 95bd26f0b2..7541ba5edb 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -267,23 +267,23 @@ test_expect_success TTY 'git config --get ignores pager.config' ' ! test -e paginated.out ' -test_expect_success TTY 'git config --get-urlmatch defaults to not paging' ' +test_expect_success TTY 'git config --get-urlmatch defaults to paging' ' rm -f paginated.out && test_terminal git -c http."https://foo.com/".bar=foo \ config --get-urlmatch http https://foo.com && - ! test -e paginated.out + test -e paginated.out ' test_expect_success TTY 'git config --get-all respects pager.config' ' rm -f paginated.out && - test_terminal git -c pager.config config --get-all foo.bar && - test -e paginated.out + test_terminal git -c pager.config=false config --get-all foo.bar && + ! test -e paginated.out ' -test_expect_success TTY 'git config --list defaults to not paging' ' +test_expect_success TTY 'git config --list defaults to paging' ' rm -f paginated.out && test_terminal git config --list && - ! test -e paginated.out + test -e paginated.out '