Re: [PATCH 5/7] tag: handle `pager.tag`-configuration within the builtin

[Jeff King]

On Mon, Jul 10, 2017 at 11:55:18PM +0200, Martin Ågren wrote:

> 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.
> 
> This is in preparation for the next patch, where we will want to handle
> slightly different configuration variables depending on which options
> are used with `git tag`. For this reason, place the call to
> setup_auto_pager() after the options have been parsed.
> 
> No functional change is intended. That said, there is a window between
> where the pager is started before and after this patch, and if an error
> occurs within this window, as of this patch the error message might not
> be paged where it would have been paged before. Since
> operation-parsing has to happen inside this window, a difference can be
> seen with, e.g., `git -c pager.tag="echo pager is used" tag
> --unknown-option`. This change in paging-behavior should be acceptable
> since it only affects erroneous usages.

Thanks for carefully thinking through the details. I agree that it's an
acceptable change of behavior.

-Peff

[PATCH 5/7] tag: handle `pager.tag`-configuration within the builtin

[Martin Ågren]

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.

This is in preparation for the next patch, where we will want to handle
slightly different configuration variables depending on which options
are used with `git tag`. For this reason, place the call to
setup_auto_pager() after the options have been parsed.

No functional change is intended. That said, there is a window between
where the pager is started before and after this patch, and if an error
occurs within this window, as of this patch the error message might not
be paged where it would have been paged before. Since
operation-parsing has to happen inside this window, a difference can be
seen with, e.g., `git -c pager.tag="echo pager is used" tag
--unknown-option`. This change in paging-behavior should be acceptable
since it only affects erroneous usages.

Signed-off-by: Martin Ågren 
---
 builtin/tag.c | 2 ++
 git.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 01154ea8d..e0f129872 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -446,6 +446,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 
argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
 
+   setup_auto_pager("tag", 0);
+
if (keyid) {
opt.sign = 1;
set_signing_key(keyid);
diff --git a/git.c b/git.c
index 696eaf87a..4d05452a3 100644
--- a/git.c
+++ b/git.c
@@ -489,7 +489,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 | IGNORE_PAGER_CONFIG },
{ "unpack-file", cmd_unpack_file, RUN_SETUP },
{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
{ "update-index", cmd_update_index, RUN_SETUP },
-- 
2.13.2.653.gfb5159d