[PATCH 2/7] git.c: let builtins opt for handling `pager.foo` themselves
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- Jul 10, 2017, 21:55 UTC
- Message-ID
- <62a5a588d880d1a41d6cdd54cd92ee577a0451fa.1499723297.git.martin.agren@gmail.com>
- In-Reply-To
- <cover.1499723297.git.martin.agren@gmail.com>
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. A user who makes use of `git tag -a` and `git tag -l` will probably choose not to configure `pager.tag` or to set it to "no", so that `git tag -a` will actually work, at the cost of not getting the pager with `git tag -l`.
To allow individual builtins to make more informed decisions about when to respect `pager.foo`, introduce a flag IGNORE_PAGER_CONFIG. If the flag is set, do not check `pager.foo`. This applies to two code-paths -- one in run_builtin() and one in execv_dashed_external().
Document the flag in api-builtin.txt.
Don't add any users of IGNORE_PAGER_CONFIG just yet, one will follow in a later patch.
Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- Documentation/technical/api-builtin.txt | 6 ++++++ git.c | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/Documentation/technical/api-builtin.txt b/Documentation/technical/api-builtin.txt index 60f442822..61fd8eeb2 100644 --- a/Documentation/technical/api-builtin.txt +++ b/Documentation/technical/api-builtin.txt @@ -46,6 +46,12 @@ where options is the bitwise-or of: The builtin supports --super-prefix. +`IGNORE_PAGER_CONFIG`:: + + Ignore the `pager.<cmd>`-configuration in git.c, instead + giving the builtin the chance to handle it and possibly + more fine-grained configurations (`pager.<cmd>.<subcmd>`). + . Add `builtin/foo.o` to `BUILTIN_OBJS` in `Makefile`. Additionally, if `foo` is a new command, there are 3 more things to do: diff --git a/git.c b/git.c index 489aab4d8..ae92f8ed5 100644 --- a/git.c +++ b/git.c @@ -283,6 +283,12 @@ static int handle_alias(int *argcp, const char ***argv) */ #define NEED_WORK_TREE (1<<3) #define SUPPORT_SUPER_PREFIX (1<<4) +/* + * Ignore the `pager.<cmd>`-configuration, instead giving the builtin + * the chance to handle it and possibly more fine-grained + * configurations (`pager.<cmd>.<subcmd>`). + */ +#define IGNORE_PAGER_CONFIG (1<<5) struct cmd_struct { const char *cmd; @@ -306,7 +312,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 & IGNORE_PAGER_CONFIG)) use_pager = check_pager_config(p->cmd); if (use_pager == -1 && p->option & USE_PAGER) use_pager = 1; @@ -543,11 +550,14 @@ static void execv_dashed_external(const char **argv) { struct child_process cmd = CHILD_PROCESS_INIT; int status; + struct cmd_struct *builtin; if (get_super_prefix()) die("%s doesn't support --super-prefix", argv[0]); - if (use_pager == -1) + builtin = get_builtin(argv[0]); + if (use_pager == -1 && + !(builtin && builtin->option & IGNORE_PAGER_CONFIG)) use_pager = check_pager_config(argv[0]); commit_pager_choice();
-- 2.13.2.653.gfb5159d