From ec14d4ecb5507550e31ba0a0016eb83fcde48f3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Wed, 2 Aug 2017 21:40:49 +0200 Subject: [PATCH 1/7] builtin.h: take over documentation from api-builtin.txt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Delete Documentation/technical/api-builtin.txt and move its content into builtin.h. Format it as a comment. Remove a '+' which was needed when the information was formatted for AsciiDoc. Similarly, change "::" to ":". Document SUPPORT_SUPER_PREFIX, thereby bringing the documentation up to date with the available flags. While at it, correct '3 more things to do' to '4 more things to do'. Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- Documentation/technical/api-builtin.txt | 73 ---------------------- builtin.h | 80 +++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 73 deletions(-) delete mode 100644 Documentation/technical/api-builtin.txt diff --git a/Documentation/technical/api-builtin.txt b/Documentation/technical/api-builtin.txt deleted file mode 100644 index 22a39b9299..0000000000 --- a/Documentation/technical/api-builtin.txt +++ /dev/null @@ -1,73 +0,0 @@ -builtin API -=========== - -Adding a new built-in ---------------------- - -There are 4 things to do to add a built-in command implementation to -Git: - -. Define the implementation of the built-in command `foo` with - signature: - - int cmd_foo(int argc, const char **argv, const char *prefix); - -. Add the external declaration for the function to `builtin.h`. - -. Add the command to the `commands[]` table defined in `git.c`. - The entry should look like: - - { "foo", cmd_foo, }, -+ -where options is the bitwise-or of: - -`RUN_SETUP`:: - If there is not a Git directory to work on, abort. If there - is a work tree, chdir to the top of it if the command was - invoked in a subdirectory. If there is no work tree, no - chdir() is done. - -`RUN_SETUP_GENTLY`:: - If there is a Git directory, chdir as per RUN_SETUP, otherwise, - don't chdir anywhere. - -`USE_PAGER`:: - - If the standard output is connected to a tty, spawn a pager and - feed our output to it. - -`NEED_WORK_TREE`:: - - Make sure there is a work tree, i.e. the command cannot act - on bare repositories. - This only makes sense when `RUN_SETUP` is also set. - -. Add `builtin/foo.o` to `BUILTIN_OBJS` in `Makefile`. - -Additionally, if `foo` is a new command, there are 3 more things to do: - -. Add tests to `t/` directory. - -. Write documentation in `Documentation/git-foo.txt`. - -. Add an entry for `git-foo` to `command-list.txt`. - -. Add an entry for `/git-foo` to `.gitignore`. - - -How a built-in is called ------------------------- - -The implementation `cmd_foo()` takes three parameters, `argc`, `argv, -and `prefix`. The first two are similar to what `main()` of a -standalone command would be called with. - -When `RUN_SETUP` is specified in the `commands[]` table, and when you -were started from a subdirectory of the work tree, `cmd_foo()` is called -after chdir(2) to the top of the work tree, and `prefix` gets the path -to the subdirectory the command started from. This allows you to -convert a user-supplied pathname (typically relative to that directory) -to a pathname relative to the top of the work tree. - -The return value from `cmd_foo()` becomes the exit status of the -command. diff --git a/builtin.h b/builtin.h index 498ac80d07..8d87d06da4 100644 --- a/builtin.h +++ b/builtin.h @@ -6,6 +6,86 @@ #include "cache.h" #include "commit.h" +/* + * builtin API + * =========== + * + * Adding a new built-in + * --------------------- + * + * There are 4 things to do to add a built-in command implementation to + * Git: + * + * . Define the implementation of the built-in command `foo` with + * signature: + * + * int cmd_foo(int argc, const char **argv, const char *prefix); + * + * . Add the external declaration for the function to `builtin.h`. + * + * . Add the command to the `commands[]` table defined in `git.c`. + * The entry should look like: + * + * { "foo", cmd_foo, }, + * + * where options is the bitwise-or of: + * + * `RUN_SETUP`: + * If there is not a Git directory to work on, abort. If there + * is a work tree, chdir to the top of it if the command was + * invoked in a subdirectory. If there is no work tree, no + * chdir() is done. + * + * `RUN_SETUP_GENTLY`: + * If there is a Git directory, chdir as per RUN_SETUP, otherwise, + * don't chdir anywhere. + * + * `USE_PAGER`: + * + * If the standard output is connected to a tty, spawn a pager and + * feed our output to it. + * + * `NEED_WORK_TREE`: + * + * Make sure there is a work tree, i.e. the command cannot act + * on bare repositories. + * This only makes sense when `RUN_SETUP` is also set. + * + * `SUPPORT_SUPER_PREFIX`: + * + * The built-in supports `--super-prefix`. + * + * . Add `builtin/foo.o` to `BUILTIN_OBJS` in `Makefile`. + * + * Additionally, if `foo` is a new command, there are 4 more things to do: + * + * . Add tests to `t/` directory. + * + * . Write documentation in `Documentation/git-foo.txt`. + * + * . Add an entry for `git-foo` to `command-list.txt`. + * + * . Add an entry for `/git-foo` to `.gitignore`. + * + * + * How a built-in is called + * ------------------------ + * + * The implementation `cmd_foo()` takes three parameters, `argc`, `argv, + * and `prefix`. The first two are similar to what `main()` of a + * standalone command would be called with. + * + * When `RUN_SETUP` is specified in the `commands[]` table, and when you + * were started from a subdirectory of the work tree, `cmd_foo()` is called + * after chdir(2) to the top of the work tree, and `prefix` gets the path + * to the subdirectory the command started from. This allows you to + * convert a user-supplied pathname (typically relative to that directory) + * to a pathname relative to the top of the work tree. + * + * The return value from `cmd_foo()` becomes the exit status of the + * command. + */ + #define DEFAULT_MERGE_LOG_LEN 20 extern const char git_usage_string[]; From c409824cc2abac46e06091fcb29639b048d23b5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Wed, 2 Aug 2017 21:40:50 +0200 Subject: [PATCH 2/7] git.c: let builtins opt for handling `pager.foo` themselves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before launching a builtin git foo and unless mechanisms with precedence are in use, we check for and handle the `pager.foo` config. This is done without considering exactly how git foo is being used, and indeed, git.c cannot (and should not) know what the arguments to git foo are supposed to achieve. In practice this means that, e.g., `git -c pager.tag tag -a new-tag` results in errors such as "Vim: Warning: Output is not to a terminal" and a garbled terminal. Someone who makes use of both `git tag -a` and `git tag -l` will probably not set `pager.tag`, so that `git tag -a` will actually work, at the cost of not paging output of `git tag -l`. To allow individual builtins to make more informed decisions about when to respect `pager.foo`, introduce a flag DELAY_PAGER_CONFIG. If the flag is set, do not check `pager.foo`. Do not check for DELAY_PAGER_CONFIG in `execv_dashed_external()`. That call site is arguably wrong, although in a way that is not yet visible, and will be changed in a slightly different direction in a later patch. Don't add any users of DELAY_PAGER_CONFIG just yet, one will follow in a later patch. Suggested-by: Jeff King Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- builtin.h | 8 ++++++++ git.c | 4 +++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/builtin.h b/builtin.h index 8d87d06da4..0f3a7b7704 100644 --- a/builtin.h +++ b/builtin.h @@ -55,6 +55,14 @@ * * The built-in supports `--super-prefix`. * + * `DELAY_PAGER_CONFIG`: + * + * If RUN_SETUP or RUN_SETUP_GENTLY is set, git.c normally handles + * the `pager.`-configuration. If this flag is used, git.c + * will skip that step, instead allowing the built-in to make a + * more informed decision, e.g., by ignoring `pager.` for + * certain subcommands. + * * . Add `builtin/foo.o` to `BUILTIN_OBJS` in `Makefile`. * * Additionally, if `foo` is a new command, there are 4 more things to do: diff --git a/git.c b/git.c index 489aab4d83..79195ebbdf 100644 --- a/git.c +++ b/git.c @@ -283,6 +283,7 @@ static int handle_alias(int *argcp, const char ***argv) */ #define NEED_WORK_TREE (1<<3) #define SUPPORT_SUPER_PREFIX (1<<4) +#define DELAY_PAGER_CONFIG (1<<5) struct cmd_struct { const char *cmd; @@ -306,7 +307,8 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) prefix = setup_git_directory_gently(&nongit_ok); } - if (use_pager == -1 && p->option & (RUN_SETUP | RUN_SETUP_GENTLY)) + if (use_pager == -1 && p->option & (RUN_SETUP | RUN_SETUP_GENTLY) && + !(p->option & DELAY_PAGER_CONFIG)) use_pager = check_pager_config(p->cmd); if (use_pager == -1 && p->option & USE_PAGER) use_pager = 1; From 033fe3d92ca16c36fb45ed7cb58d42344088e7bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Wed, 2 Aug 2017 21:40:51 +0200 Subject: [PATCH 3/7] git.c: provide setup_auto_pager() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous patch introduced a way for builtins to declare that they will take responsibility for handling the `pager.foo`-config item. (See the commit message of that patch for why that could be useful.) Provide setup_auto_pager(), which builtins can call in order to handle `pager.`, including possibly starting the pager. Make this function don't do anything if a pager has already been started, as indicated by use_pager or pager_in_use(). Whenever this function is called from a builtin, git.c will already have called commit_pager_choice(). Since commit_pager_choice() treats the special value -1 as "punt" or "not yet decided", it is not a problem that we might end up calling commit_pager_choice() once in git.c and once (or more) in the builtin. Make the new function use -1 in the same way and document it as "punt". Don't add any users of setup_auto_pager just yet, one will follow in a later patch. Suggested-by: Jeff King Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- builtin.h | 12 ++++++++++++ git.c | 10 ++++++++++ 2 files changed, 22 insertions(+) diff --git a/builtin.h b/builtin.h index 0f3a7b7704..42378f3aa4 100644 --- a/builtin.h +++ b/builtin.h @@ -113,6 +113,18 @@ struct fmt_merge_msg_opts { extern int fmt_merge_msg(struct strbuf *in, struct strbuf *out, struct fmt_merge_msg_opts *); +/** + * If a built-in has DELAY_PAGER_CONFIG set, the built-in should call this early + * when it wishes to respect the `pager.foo`-config. The `cmd` is the name of + * the built-in, e.g., "foo". If a paging-choice has already been setup, this + * does nothing. The default in `def` should be 0 for "pager off", 1 for "pager + * on" or -1 for "punt". + * + * You should most likely use a default of 0 or 1. "Punt" (-1) could be useful + * to be able to fall back to some historical compatibility name. + */ +extern void setup_auto_pager(const char *cmd, int def); + extern int is_builtin(const char *s); extern int cmd_add(int argc, const char **argv, const char *prefix); diff --git a/git.c b/git.c index 79195ebbdf..66832f2324 100644 --- a/git.c +++ b/git.c @@ -33,6 +33,16 @@ static void commit_pager_choice(void) { } } +void setup_auto_pager(const char *cmd, int def) +{ + if (use_pager != -1 || pager_in_use()) + return; + use_pager = check_pager_config(cmd); + if (use_pager == -1) + use_pager = def; + commit_pager_choice(); +} + static int handle_options(const char ***argv, int *argc, int *envchanged) { const char **orig_argv = *argv; From b3ee740c8275675a97974bcb27a18eb7997fa907 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Wed, 2 Aug 2017 21:40:52 +0200 Subject: [PATCH 4/7] t7006: add tests for how git tag paginates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using, e.g., `git -c pager.tag tag -a new-tag` results in errors such as "Vim: Warning: Output is not to a terminal" and a garbled terminal. Someone who makes use of both `git tag -a` and `git tag -l` will probably not set `pager.tag`, so that `git tag -a` will actually work, at the cost of not paging output of `git tag -l`. Since we're about to change how `git tag` respects `pager.tag`, add tests around this, including how the configuration is ignored if --no-pager or --paginate are used. Construct tests with a few different subcommands. First, use -l. Second, use "no arguments" and --contains, since those imply -l. (There are more arguments which imply -l, but using these two should be enough.) Third, use -a as a representative for "not -l". Actually, the tests use `git tag -am` so no editor is launched, but that is irrelevant, since we just want to see whether the pager is used or not. Make one of the tests demonstrate the broken behavior mentioned above, where `git tag -a` respects `pager.tag`. Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- t/t7006-pager.sh | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index 20b4d83c28..b56d4cdd41 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -134,6 +134,73 @@ test_expect_success TTY 'configuration can enable pager (from subdir)' ' } ' +test_expect_success TTY 'git tag -l defaults to not paging' ' + rm -f paginated.out && + test_terminal git tag -l && + ! test -e paginated.out +' + +test_expect_success TTY 'git tag -l respects pager.tag' ' + rm -f paginated.out && + test_terminal git -c pager.tag tag -l && + test -e paginated.out +' + +test_expect_success TTY 'git tag -l respects --no-pager' ' + rm -f paginated.out && + test_terminal git -c pager.tag --no-pager tag -l && + ! test -e paginated.out +' + +test_expect_success TTY 'git tag with no args defaults to not paging' ' + # no args implies -l so this should page like -l + rm -f paginated.out && + test_terminal git tag && + ! test -e paginated.out +' + +test_expect_success TTY 'git tag with no args respects pager.tag' ' + # no args implies -l so this should page like -l + rm -f paginated.out && + test_terminal git -c pager.tag tag && + test -e paginated.out +' + +test_expect_success TTY 'git tag --contains defaults to not paging' ' + # --contains implies -l so this should page like -l + rm -f paginated.out && + test_terminal git tag --contains && + ! test -e paginated.out +' + +test_expect_success TTY 'git tag --contains respects pager.tag' ' + # --contains implies -l so this should page like -l + rm -f paginated.out && + test_terminal git -c pager.tag tag --contains && + test -e paginated.out +' + +test_expect_success TTY 'git tag -a defaults to not paging' ' + test_when_finished "git tag -d newtag" && + rm -f paginated.out && + test_terminal git tag -am message newtag && + ! test -e paginated.out +' + +test_expect_failure TTY 'git tag -a ignores pager.tag' ' + test_when_finished "git tag -d newtag" && + rm -f paginated.out && + test_terminal git -c pager.tag tag -am message newtag && + ! test -e paginated.out +' + +test_expect_success TTY 'git tag -a respects --paginate' ' + test_when_finished "git tag -d newtag" && + rm -f paginated.out && + test_terminal git --paginate tag -am message newtag && + 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 de121ffe57fd14334c24f0ac51dbc6828a3bc315 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Wed, 2 Aug 2017 21:40:53 +0200 Subject: [PATCH 5/7] tag: respect `pager.tag` in list-mode only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using, e.g., `git -c pager.tag tag -a new-tag` results in errors such as "Vim: Warning: Output is not to a terminal" and a garbled terminal. Someone who makes use of both `git tag -a` and `git tag -l` will probably not set `pager.tag`, so that `git tag -a` will actually work, at the cost of not paging output of `git tag -l`. Use the mechanisms introduced in two earlier patches to ignore `pager.tag` in git.c and let the `git tag` builtin handle it on its own. Only respect `pager.tag` when running in list-mode. There is a window between where the pager is started before and after this patch. This means that early errors can behave slightly different before and after this patch. Since operation-parsing has to happen inside this window, this can be seen with `git -c pager.tag="echo pager is used" tag -l --unknown-option`. This change in paging-behavior should be acceptable since it only affects erroneous usages. Update the documentation and update tests. If an alias is used to run `git tag -a`, then `pager.tag` will still be respected. Document this known breakage. It will be fixed in a later commit. Add a similar test for `-l`, which works. Noticed-by: Anatoly Borodin Suggested-by: Jeff King Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- Documentation/git-tag.txt | 3 +++ builtin/tag.c | 3 +++ git.c | 2 +- t/t7006-pager.sh | 15 ++++++++++++++- 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index 1eb15afa1c..875d135e0e 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -205,6 +205,9 @@ it in the repository configuration as follows: signingKey = ------------------------------------- +`pager.tag` is only respected when listing tags, i.e., when `-l` is +used or implied. +See linkgit:git-config[1]. DISCUSSION ---------- diff --git a/builtin/tag.c b/builtin/tag.c index 01154ea8dc..5ad1af2524 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -461,6 +461,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix) cmdmode = 'l'; } + if (cmdmode == 'l') + setup_auto_pager("tag", 0); + if ((create_tag_object || force) && (cmdmode != 0)) usage_with_options(git_tag_usage, options); diff --git a/git.c b/git.c index 66832f2324..82ac2a092c 100644 --- a/git.c +++ b/git.c @@ -466,7 +466,7 @@ static struct cmd_struct commands[] = { { "stripspace", cmd_stripspace }, { "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX}, { "symbolic-ref", cmd_symbolic_ref, RUN_SETUP }, - { "tag", cmd_tag, RUN_SETUP }, + { "tag", cmd_tag, RUN_SETUP | DELAY_PAGER_CONFIG }, { "unpack-file", cmd_unpack_file, RUN_SETUP }, { "unpack-objects", cmd_unpack_objects, RUN_SETUP }, { "update-index", cmd_update_index, RUN_SETUP }, diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index b56d4cdd41..570b2f252d 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -187,7 +187,7 @@ test_expect_success TTY 'git tag -a defaults to not paging' ' ! test -e paginated.out ' -test_expect_failure TTY 'git tag -a ignores pager.tag' ' +test_expect_success TTY 'git tag -a ignores pager.tag' ' test_when_finished "git tag -d newtag" && rm -f paginated.out && test_terminal git -c pager.tag tag -am message newtag && @@ -201,6 +201,19 @@ test_expect_success TTY 'git tag -a respects --paginate' ' test -e paginated.out ' +test_expect_failure TTY 'git tag as alias ignores pager.tag with -a' ' + test_when_finished "git tag -d newtag" && + rm -f paginated.out && + test_terminal git -c pager.tag -c alias.t=tag t -am message newtag && + ! test -e paginated.out +' + +test_expect_success TTY 'git tag as alias respects pager.tag with -l' ' + rm -f paginated.out && + test_terminal git -c pager.tag -c alias.t=tag t -l && + 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 ff1e72483f416ecb58737205163031f54c24e754 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Wed, 2 Aug 2017 21:40:54 +0200 Subject: [PATCH 6/7] tag: change default of `pager.tag` to "on" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous patch taught `git tag` to only respect `pager.tag` in list-mode. That patch left the default value of `pager.tag` at "off". After that patch, it makes sense to let the default value be "on" instead, since it will help with listing many tags, but will not hurt users of `git tag -a` as it would have before. Make that change. Update documentation and tests. Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- Documentation/git-tag.txt | 2 +- builtin/tag.c | 2 +- t/t7006-pager.sh | 28 ++++++++++++++-------------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index 875d135e0e..d97aad3439 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -206,7 +206,7 @@ it in the repository configuration as follows: ------------------------------------- `pager.tag` is only respected when listing tags, i.e., when `-l` is -used or implied. +used or implied. The default is to use a pager. See linkgit:git-config[1]. DISCUSSION diff --git a/builtin/tag.c b/builtin/tag.c index 5ad1af2524..ea83df5e13 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -462,7 +462,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) } if (cmdmode == 'l') - setup_auto_pager("tag", 0); + setup_auto_pager("tag", 1); if ((create_tag_object || force) && (cmdmode != 0)) usage_with_options(git_tag_usage, options); diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index 570b2f252d..afa03f3b65 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -134,16 +134,16 @@ test_expect_success TTY 'configuration can enable pager (from subdir)' ' } ' -test_expect_success TTY 'git tag -l defaults to not paging' ' +test_expect_success TTY 'git tag -l defaults to paging' ' rm -f paginated.out && test_terminal git tag -l && - ! test -e paginated.out + test -e paginated.out ' test_expect_success TTY 'git tag -l respects pager.tag' ' rm -f paginated.out && - test_terminal git -c pager.tag tag -l && - test -e paginated.out + test_terminal git -c pager.tag=false tag -l && + ! test -e paginated.out ' test_expect_success TTY 'git tag -l respects --no-pager' ' @@ -152,32 +152,32 @@ test_expect_success TTY 'git tag -l respects --no-pager' ' ! test -e paginated.out ' -test_expect_success TTY 'git tag with no args defaults to not paging' ' +test_expect_success TTY 'git tag with no args defaults to paging' ' # no args implies -l so this should page like -l rm -f paginated.out && test_terminal git tag && - ! test -e paginated.out + test -e paginated.out ' test_expect_success TTY 'git tag with no args respects pager.tag' ' # no args implies -l so this should page like -l rm -f paginated.out && - test_terminal git -c pager.tag tag && - test -e paginated.out + test_terminal git -c pager.tag=false tag && + ! test -e paginated.out ' -test_expect_success TTY 'git tag --contains defaults to not paging' ' +test_expect_success TTY 'git tag --contains defaults to paging' ' # --contains implies -l so this should page like -l rm -f paginated.out && test_terminal git tag --contains && - ! test -e paginated.out + test -e paginated.out ' test_expect_success TTY 'git tag --contains respects pager.tag' ' # --contains implies -l so this should page like -l rm -f paginated.out && - test_terminal git -c pager.tag tag --contains && - test -e paginated.out + test_terminal git -c pager.tag=false tag --contains && + ! test -e paginated.out ' test_expect_success TTY 'git tag -a defaults to not paging' ' @@ -210,8 +210,8 @@ test_expect_failure TTY 'git tag as alias ignores pager.tag with -a' ' test_expect_success TTY 'git tag as alias respects pager.tag with -l' ' rm -f paginated.out && - test_terminal git -c pager.tag -c alias.t=tag t -l && - test -e paginated.out + test_terminal git -c pager.tag=false -c alias.t=tag t -l && + ! test -e paginated.out ' # A colored commit log will begin with an appropriate ANSI escape From 595d59e2b53a19f8c5c277348e4e1a07bb913ba4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Wed, 2 Aug 2017 21:40:55 +0200 Subject: [PATCH 7/7] git.c: ignore pager.* when launching builtin as dashed external MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When running, e.g., `git -c alias.bar=foo bar`, we expand the alias and execute `git-foo` as a dashed external. This is true even if git foo is a builtin. That is on purpose, and is motivated in a comment which was added in commit 441981bc ("git: simplify environment save/restore logic", 2016-01-26). Shortly before we launch a dashed external, and unless we have already found out whether we should use a pager, we check `pager.foo`. This was added in commit 92058e4d ("support pager.* for external commands", 2011-08-18). If the dashed external is a builtin, this does not match that commit's intention and is arguably wrong, since it would be cleaner if we let the "dashed external builtin" handle `pager.foo`. This has not mattered in practice, but a recent patch taught `git-tag` to ignore `pager.tag` under certain circumstances. But, when started using an alias, it doesn't get the chance to do so, as outlined above. That recent patch added a test to document this breakage. Do not check `pager.foo` before launching a builtin as a dashed external, i.e., if we recognize the name of the external as a builtin. Change the test to use `test_expect_success`. Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- git.c | 2 +- t/t7006-pager.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/git.c b/git.c index 82ac2a092c..6b6d9f68e1 100644 --- a/git.c +++ b/git.c @@ -559,7 +559,7 @@ static void execv_dashed_external(const char **argv) if (get_super_prefix()) die("%s doesn't support --super-prefix", argv[0]); - if (use_pager == -1) + if (use_pager == -1 && !is_builtin(argv[0])) use_pager = check_pager_config(argv[0]); commit_pager_choice(); diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index afa03f3b65..9128ec5acd 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -201,7 +201,7 @@ test_expect_success TTY 'git tag -a respects --paginate' ' test -e paginated.out ' -test_expect_failure TTY 'git tag as alias ignores pager.tag with -a' ' +test_expect_success TTY 'git tag as alias ignores pager.tag with -a' ' test_when_finished "git tag -d newtag" && rm -f paginated.out && test_terminal git -c pager.tag -c alias.t=tag t -am message newtag &&