Re: [PATCH] show git tag output in pager

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 29, 2011 at 11:37:49AM +0200, Michal Vyskocil wrote:

> On Tue, Sep 27, 2011 at 04:19:39PM +0200, Matthieu Moy wrote:
> > The commit message should explain why this is needed, and in particular
> > why you prefer this to setting pager.tag in your ~/.gitconfig.
> 
> Opps! I read a documentation, but I did not realize this works for all
> commands and not only for them calling setup_pager(). Then sorry, no
> change is needed.

I don't think you want to set pager.tag. It will invoke the pager for
all tag subcommands, including tag creation and deletion. It's not a
huge deal if your pager exits immediately when the input is less than a
page (which I think our default LESS settings will do). But I wouldn't
be surprised if it ends up confusing some program at some point.

I think instead, you want some way for commands to say "OK, I'm in a
subcommand that might or might not want a pager now".

Something like the (thoroughly not tested) patch below, which you can
use like:

  git config pager.tag.list

diff --git a/builtin.h b/builtin.h
index 0e9da90..d0fe971 100644
--- a/builtin.h
+++ b/builtin.h
@@ -35,6 +35,7 @@ int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
 void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c);
 
 extern int check_pager_config(const char *cmd);
+extern void try_subcommand_pager(const char *subcommand);
 
 extern int textconv_object(const char *path, unsigned mode, const unsigned char *sha1, char **buf, unsigned long *buf_size);
 
diff --git a/builtin/tag.c b/builtin/tag.c
index 667515e..bc45fe6 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -152,6 +152,7 @@ static int list_tags(const char **patterns, int lines,
 	filter.lines = lines;
 	filter.with_commit = with_commit;
 
+	try_subcommand_pager("tag.list");
 	for_each_tag_ref(show_reference, (void *) &filter);
 
 	return 0;
diff --git a/git.c b/git.c
index 8e34903..60fbf1e 100644
--- a/git.c
+++ b/git.c
@@ -64,6 +64,16 @@ static void commit_pager_choice(void) {
 	}
 }
 
+void try_subcommand_pager(const char *subcommand)
+{
+	/* it's too late to turn off a running pager */
+	if (pager_in_use())
+		return;
+
+	use_pager = check_pager_config(subcommand);
+	commit_pager_choice();
+}
+
 static int handle_options(const char ***argv, int *argc, int *envchanged)
 {
 	const char **orig_argv = *argv;

Other programs like "git branch list" would probably want similar hooks.

For commands like "tag" and "branch" where there's really only one
operation that you would want to be paged (i.e., "list"), it's tempting
to also (or instead) make "pager.tag" only affect the useful command.

That's not too hard for internal commands like "tag" (patch below). But
I'm not sure how you would do it for something external like "stash";
from git.c's perspective, we don't know anything about stash or whether
it would want to respect pager config or not.

diff --git a/builtin/tag.c b/builtin/tag.c
index bc45fe6..bbdb135 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -152,7 +152,7 @@ static int list_tags(const char **patterns, int lines,
 	filter.lines = lines;
 	filter.with_commit = with_commit;
 
-	try_subcommand_pager("tag.list");
+	try_subcommand_pager("tag");
 	for_each_tag_ref(show_reference, (void *) &filter);
 
 	return 0;
diff --git a/git.c b/git.c
index 60fbf1e..64a078d 100644
--- a/git.c
+++ b/git.c
@@ -276,6 +276,7 @@ static int handle_alias(int *argcp, const char ***argv)
  * RUN_SETUP for reading from the configuration file.
  */
 #define NEED_WORK_TREE		(1<<3)
+#define NO_PAGER_CONFIG		(1<<4)
 
 struct cmd_struct {
 	const char *cmd;
@@ -299,7 +300,9 @@ 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 & NO_PAGER_CONFIG))
 			use_pager = check_pager_config(p->cmd);
 		if (use_pager == -1 && p->option & USE_PAGER)
 			use_pager = 1;
@@ -436,7 +439,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
 		{ "stripspace", cmd_stripspace },
 		{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
-		{ "tag", cmd_tag, RUN_SETUP },
+		{ "tag", cmd_tag, RUN_SETUP | NO_PAGER_CONFIG },
 		{ "tar-tree", cmd_tar_tree },
 		{ "unpack-file", cmd_unpack_file, RUN_SETUP },
 		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]