Re: [PATCH v2 08/10] tag: respect `pager.tag` in list-mode only

2017-07-30 Thread Jeff King
On Mon, Jul 17, 2017 at 10:10:50PM +0200, Martin Ågren wrote:

>  test_expect_success TTY 'git tag -a respects --paginate' '
>   test_when_finished "git tag -d newtag" &&
>   rm -f paginated.out &&
> - test_terminal git -c pager.tag=false --paginate \
> - tag -am message newtag &&
> + test_terminal git --paginate tag -am message newtag &&
> + test -e paginated.out
> +'

This changes, I guess, because pager.tag should not be having any impact
at all. So it would not hurt to leave it, but the in the final state of
the test what is interesting is that "--paginate" kicks in. I do wonder
if it could just drop the pager.tag bit when it is added in the first
place. We test elsewhere that the command-line overrides the config (for
a case that actually _does_ respect the config).

I'm OK with it either way, though.

-Peff


[PATCH v2 08/10] tag: respect `pager.tag` in list-mode only

2017-07-17 Thread Martin Ågren
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. 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`.

Teach git tag to only respect `pager.tag` when running in list-mode.
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 as it should.

Noticed-by: Anatoly Borodin 
Suggested-by: Jeff King 
Signed-off-by: Martin Ågren 
---
 Documentation/git-tag.txt |  3 +++
 t/t7006-pager.sh  | 25 +
 builtin/tag.c |  3 ++-
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 1eb15afa1..875d135e0 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/t/t7006-pager.sh b/t/t7006-pager.sh
index e7430bc93..a357436e1 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -187,18 +187,35 @@ test_expect_success TTY 'git tag -a defaults to not 
paging' '
! test -e paginated.out
 '
 
-test_expect_success TTY 'git tag -a respects 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 &&
-   test -e paginated.out
+   ! 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 -c pager.tag=false --paginate \
-   tag -am message newtag &&
+   test_terminal git --paginate tag -am message newtag &&
+   test -e paginated.out
+'
+
+test_expect_failure TTY 'git tag as alias ignores pager.tag with -a' '
+   # git-tag will be launched as a dashed external, which
+   # 1) is the source of a potential bug, and
+   # 2) is why we use test_config and not -c.
+   test_when_finished "git tag -d newtag" &&
+   rm -f paginated.out &&
+   test_config pager.tag true &&
+   test_terminal git -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_config pager.tag true &&
+   test_terminal git -c alias.t=tag t -l &&
test -e paginated.out
 '
 
diff --git a/builtin/tag.c b/builtin/tag.c
index 9eda434fb..5ad1af252 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -461,7 +461,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
cmdmode = 'l';
}
 
-   setup_auto_pager("tag", 0);
+   if (cmdmode == 'l')
+   setup_auto_pager("tag", 0);
 
if ((create_tag_object || force) && (cmdmode != 0))
usage_with_options(git_tag_usage, options);
-- 
2.14.0.rc0