{"thread":{"id":"28510","subject":"[PATCH] show git tag output in pager","startedAt":"2011-09-27T13:42:36Z","lastAt":"2011-10-07T16:16:48Z","messageCount":8,"participants":["Michal Vyskocil","Matthieu Moy","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"176331","messageId":"20110927134235.GA27478@zelva.suse.cz","threadId":"28510","inReplyTo":null,"subject":"[PATCH] show git tag output in pager","fromName":"Michal Vyskocil","fromEmail":"mvyskocil@suse.cz","sentAt":"2011-09-27T13:42:36Z","receivedAt":"2011-09-27T13:42:36Z","isPatch":true,"sender":{"key":"mvyskocil@suse.cz","avatar":null},"body":"---\n builtin/tag.c |    2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 667515e..9f70fa8 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -147,6 +147,8 @@ static int list_tags(const char **patterns, int lines,\n \t\t\tstruct commit_list *with_commit)\n {\n \tstruct tag_filter filter;\n+        \n+        setup_pager();\n \n \tfilter.patterns = patterns;\n \tfilter.lines = lines;\n-- \n1.7.6.3\n\n"},{"id":"176332","messageId":"vpqehz2cbk4.fsf@bauges.imag.fr","threadId":"28510","inReplyTo":"20110927134235.GA27478@zelva.suse.cz","subject":"Re: [PATCH] show git tag output in pager","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2011-09-27T14:19:39Z","receivedAt":"2011-09-27T14:19:39Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"The commit message should explain why this is needed, and in particular\nwhy you prefer this to setting pager.tag in your ~/.gitconfig.\n\nMichal Vyskocil <mvyskocil@suse.cz> writes:\n\n> --- a/builtin/tag.c\n> +++ b/builtin/tag.c\n> @@ -147,6 +147,8 @@ static int list_tags(const char **patterns, int lines,\n>  \t\t\tstruct commit_list *with_commit)\n>  {\n>  \tstruct tag_filter filter;\n> +        \n> +        setup_pager();\n\nGit indents with tabs, not spaces, and does not leave trailing\nwhitespaces.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"176478","messageId":"20110929093749.GB27152@zelva.suse.cz","threadId":"28510","inReplyTo":"vpqehz2cbk4.fsf@bauges.imag.fr","subject":"Re: [PATCH] show git tag output in pager","fromName":"Michal Vyskocil","fromEmail":"mvyskocil@suse.cz","sentAt":"2011-09-29T09:37:49Z","receivedAt":"2011-09-29T09:37:49Z","isPatch":true,"sender":{"key":"mvyskocil@suse.cz","avatar":null},"body":"On Tue, Sep 27, 2011 at 04:19:39PM +0200, Matthieu Moy wrote:\n> The commit message should explain why this is needed, and in particular\n> why you prefer this to setting pager.tag in your ~/.gitconfig.\n\nOpps! I read a documentation, but I did not realize this works for all\ncommands and not only for them calling setup_pager(). Then sorry, no\nchange is needed.\n\n> \n> Michal Vyskocil <mvyskocil@suse.cz> writes:\n> \n> > --- a/builtin/tag.c\n> > +++ b/builtin/tag.c\n> > @@ -147,6 +147,8 @@ static int list_tags(const char **patterns, int lines,\n> >  \t\t\tstruct commit_list *with_commit)\n> >  {\n> >  \tstruct tag_filter filter;\n> > +        \n> > +        setup_pager();\n> \n> Git indents with tabs, not spaces, and does not leave trailing\n> whitespaces.\n\nOK, thanks for the feedbac. I will try to be more carefull in the\nfuture.\n\nRegards\nMichal Vyskocil\n\n> \n> -- \n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n"},{"id":"176563","messageId":"20110930104241.GB24507@sigill.intra.peff.net","threadId":"28510","inReplyTo":"20110929093749.GB27152@zelva.suse.cz","subject":"Re: [PATCH] show git tag output in pager","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-30T10:42:41Z","receivedAt":"2011-09-30T10:42:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 29, 2011 at 11:37:49AM +0200, Michal Vyskocil wrote:\n\n> On Tue, Sep 27, 2011 at 04:19:39PM +0200, Matthieu Moy wrote:\n> > The commit message should explain why this is needed, and in particular\n> > why you prefer this to setting pager.tag in your ~/.gitconfig.\n> \n> Opps! I read a documentation, but I did not realize this works for all\n> commands and not only for them calling setup_pager(). Then sorry, no\n> change is needed.\n\nI don't think you want to set pager.tag. It will invoke the pager for\nall tag subcommands, including tag creation and deletion. It's not a\nhuge deal if your pager exits immediately when the input is less than a\npage (which I think our default LESS settings will do). But I wouldn't\nbe surprised if it ends up confusing some program at some point.\n\nI think instead, you want some way for commands to say \"OK, I'm in a\nsubcommand that might or might not want a pager now\".\n\nSomething like the (thoroughly not tested) patch below, which you can\nuse like:\n\n  git config pager.tag.list\n\ndiff --git a/builtin.h b/builtin.h\nindex 0e9da90..d0fe971 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -35,6 +35,7 @@ int copy_note_for_rewrite(struct notes_rewrite_cfg *c,\n void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c);\n \n extern int check_pager_config(const char *cmd);\n+extern void try_subcommand_pager(const char *subcommand);\n \n extern int textconv_object(const char *path, unsigned mode, const unsigned char *sha1, char **buf, unsigned long *buf_size);\n \ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 667515e..bc45fe6 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -152,6 +152,7 @@ static int list_tags(const char **patterns, int lines,\n \tfilter.lines = lines;\n \tfilter.with_commit = with_commit;\n \n+\ttry_subcommand_pager(\"tag.list\");\n \tfor_each_tag_ref(show_reference, (void *) &filter);\n \n \treturn 0;\ndiff --git a/git.c b/git.c\nindex 8e34903..60fbf1e 100644\n--- a/git.c\n+++ b/git.c\n@@ -64,6 +64,16 @@ static void commit_pager_choice(void) {\n \t}\n }\n \n+void try_subcommand_pager(const char *subcommand)\n+{\n+\t/* it's too late to turn off a running pager */\n+\tif (pager_in_use())\n+\t\treturn;\n+\n+\tuse_pager = check_pager_config(subcommand);\n+\tcommit_pager_choice();\n+}\n+\n static int handle_options(const char ***argv, int *argc, int *envchanged)\n {\n \tconst char **orig_argv = *argv;\n\nOther programs like \"git branch list\" would probably want similar hooks.\n\nFor commands like \"tag\" and \"branch\" where there's really only one\noperation that you would want to be paged (i.e., \"list\"), it's tempting\nto also (or instead) make \"pager.tag\" only affect the useful command.\n\nThat's not too hard for internal commands like \"tag\" (patch below). But\nI'm not sure how you would do it for something external like \"stash\";\nfrom git.c's perspective, we don't know anything about stash or whether\nit would want to respect pager config or not.\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex bc45fe6..bbdb135 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -152,7 +152,7 @@ static int list_tags(const char **patterns, int lines,\n \tfilter.lines = lines;\n \tfilter.with_commit = with_commit;\n \n-\ttry_subcommand_pager(\"tag.list\");\n+\ttry_subcommand_pager(\"tag\");\n \tfor_each_tag_ref(show_reference, (void *) &filter);\n \n \treturn 0;\ndiff --git a/git.c b/git.c\nindex 60fbf1e..64a078d 100644\n--- a/git.c\n+++ b/git.c\n@@ -276,6 +276,7 @@ static int handle_alias(int *argcp, const char ***argv)\n  * RUN_SETUP for reading from the configuration file.\n  */\n #define NEED_WORK_TREE\t\t(1<<3)\n+#define NO_PAGER_CONFIG\t\t(1<<4)\n \n struct cmd_struct {\n \tconst char *cmd;\n@@ -299,7 +300,9 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)\n \t\t\tprefix = setup_git_directory_gently(&nongit_ok);\n \t\t}\n \n-\t\tif (use_pager == -1 && p->option & (RUN_SETUP | RUN_SETUP_GENTLY))\n+\t\tif (use_pager == -1 &&\n+\t\t    p->option & (RUN_SETUP | RUN_SETUP_GENTLY) &&\n+\t\t    !(p->option & NO_PAGER_CONFIG))\n \t\t\tuse_pager = check_pager_config(p->cmd);\n \t\tif (use_pager == -1 && p->option & USE_PAGER)\n \t\t\tuse_pager = 1;\n@@ -436,7 +439,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"status\", cmd_status, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"stripspace\", cmd_stripspace },\n \t\t{ \"symbolic-ref\", cmd_symbolic_ref, RUN_SETUP },\n-\t\t{ \"tag\", cmd_tag, RUN_SETUP },\n+\t\t{ \"tag\", cmd_tag, RUN_SETUP | NO_PAGER_CONFIG },\n \t\t{ \"tar-tree\", cmd_tar_tree },\n \t\t{ \"unpack-file\", cmd_unpack_file, RUN_SETUP },\n \t\t{ \"unpack-objects\", cmd_unpack_objects, RUN_SETUP },\n\n-Peff\n"},{"id":"176741","messageId":"vpqwrcmw7ve.fsf@bauges.imag.fr","threadId":"28510","inReplyTo":"20110930104241.GB24507@sigill.intra.peff.net","subject":"Re: [PATCH] show git tag output in pager","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2011-10-03T12:57:09Z","receivedAt":"2011-10-03T12:57:09Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Sep 29, 2011 at 11:37:49AM +0200, Michal Vyskocil wrote:\n>\n>> On Tue, Sep 27, 2011 at 04:19:39PM +0200, Matthieu Moy wrote:\n>> > The commit message should explain why this is needed, and in particular\n>> > why you prefer this to setting pager.tag in your ~/.gitconfig.\n>> \n>> Opps! I read a documentation, but I did not realize this works for all\n>> commands and not only for them calling setup_pager(). Then sorry, no\n>> change is needed.\n>\n> I don't think you want to set pager.tag. It will invoke the pager for\n> all tag subcommands, including tag creation and deletion.\n\nThat's the kind of argument/discussion I was expecting in the commit\nmessage.\n\n> I think instead, you want some way for commands to say \"OK, I'm in a\n> subcommand that might or might not want a pager now\".\n\nRight.\n\nI like the try_subcommand_pager idea. Ideally, there would also be a\nnice mechanism to set defaults for subcommands, so that \"git tag\n<whatever>\" does the right thing without configuration.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"177153","messageId":"20111007144438.GA30318@sigill.intra.peff.net","threadId":"28510","inReplyTo":"vpqwrcmw7ve.fsf@bauges.imag.fr","subject":"Re: [PATCH] show git tag output in pager","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-07T14:44:38Z","receivedAt":"2011-10-07T14:44:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 03, 2011 at 02:57:09PM +0200, Matthieu Moy wrote:\n\n> I like the try_subcommand_pager idea. Ideally, there would also be a\n> nice mechanism to set defaults for subcommands, so that \"git tag\n> <whatever>\" does the right thing without configuration.\n\nThat's easy enough. Something like the patch below?\n\nStill thoroughly untested, but it looks Obviously Correct to me. :)\n\n---\n builtin.h     |    1 +\n builtin/tag.c |    1 +\n git.c         |   13 +++++++++++++\n 3 files changed, 15 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin.h b/builtin.h\nindex 0e9da90..b2badf8 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -35,6 +35,7 @@ int copy_note_for_rewrite(struct notes_rewrite_cfg *c,\n void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c);\n \n extern int check_pager_config(const char *cmd);\n+extern void try_subcommand_pager(const char *subcommand, int fallback);\n \n extern int textconv_object(const char *path, unsigned mode, const unsigned char *sha1, char **buf, unsigned long *buf_size);\n \ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 9d89616..c77adc4 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -152,6 +152,7 @@ static int list_tags(const char **patterns, int lines,\n \tfilter.lines = lines;\n \tfilter.with_commit = with_commit;\n \n+\ttry_subcommand_pager(\"tag.list\", 1);\n \tfor_each_tag_ref(show_reference, (void *) &filter);\n \n \treturn 0;\ndiff --git a/git.c b/git.c\nindex 8e34903..11ee1a8 100644\n--- a/git.c\n+++ b/git.c\n@@ -64,6 +64,19 @@ static void commit_pager_choice(void) {\n \t}\n }\n \n+void try_subcommand_pager(const char *subcommand, int fallback)\n+{\n+\t/* it's too late to turn off a running pager */\n+\tif (pager_in_use())\n+\t\treturn;\n+\n+\tif (use_pager == -1)\n+\t\tuse_pager = check_pager_config(subcommand);\n+\tif (use_pager == -1)\n+\t\tuse_pager = fallback;\n+\tcommit_pager_choice();\n+}\n+\n static int handle_options(const char ***argv, int *argc, int *envchanged)\n {\n \tconst char **orig_argv = *argv;\n-- \n1.7.7.rc2.7.gdfc92\n"},{"id":"177154","messageId":"vpqsjn4dfi4.fsf@bauges.imag.fr","threadId":"28510","inReplyTo":"20111007144438.GA30318@sigill.intra.peff.net","subject":"Re: [PATCH] show git tag output in pager","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2011-10-07T14:48:35Z","receivedAt":"2011-10-07T14:48:35Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Oct 03, 2011 at 02:57:09PM +0200, Matthieu Moy wrote:\n>\n>> I like the try_subcommand_pager idea. Ideally, there would also be a\n>> nice mechanism to set defaults for subcommands, so that \"git tag\n>> <whatever>\" does the right thing without configuration.\n>\n> That's easy enough. Something like the patch below?\n\nIt may have been better with a big centralized array of configurations\nlike\n\n\tstatic struct cmd_struct commands[] = {\n\t\t{ \"add\", cmd_add, RUN_SETUP | NEED_WORK_TREE },\n                ...\n\nin git.c, but if we have only a few instances of this, your system is\nprobably fine. I like it.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"177159","messageId":"20111007161648.GA4399@sigill.intra.peff.net","threadId":"28510","inReplyTo":"vpqsjn4dfi4.fsf@bauges.imag.fr","subject":"Re: [PATCH] show git tag output in pager","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-07T16:16:48Z","receivedAt":"2011-10-07T16:16:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 07, 2011 at 04:48:35PM +0200, Matthieu Moy wrote:\n\n> > On Mon, Oct 03, 2011 at 02:57:09PM +0200, Matthieu Moy wrote:\n> >\n> >> I like the try_subcommand_pager idea. Ideally, there would also be a\n> >> nice mechanism to set defaults for subcommands, so that \"git tag\n> >> <whatever>\" does the right thing without configuration.\n> >\n> > That's easy enough. Something like the patch below?\n> \n> It may have been better with a big centralized array of configurations\n> like\n> \n> \tstatic struct cmd_struct commands[] = {\n> \t\t{ \"add\", cmd_add, RUN_SETUP | NEED_WORK_TREE },\n>                 ...\n> \n> in git.c, but if we have only a few instances of this, your system is\n> probably fine. I like it.\n\nI don't think you can centralize it in the same way, because some of it\nwill have to be implemented in shell script (unlike the full-command\nones, which we can always trigger at the git.c wrapper layer).\n\nSo you could have:\n\n  static struct pager_default subcommands[] = {\n          { \"tag.list\", 1 },\n          { \"branch.list\", 1 },\n  };\n\nbut you'll never be able to put \"stash.list\" into that structure (and as\nyou probably guessed, my patch isn't enough to implement stash.list,\neither; it would need a shell implementation of try_subcommand_pager).\n\n-Peff\n"}]}