Re: [PATCH/RFC 0/9] Setup cleanup, chapter one
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Apr 15, 2010, 00:05 UTC
- Message-ID
- <20100415000448.GA14151@progeny.tock>
- In-Reply-To
- <7vwrw9oiva.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
Show 8 quoted lines
> By the way, the other day I had an interesting experience after running: > > git -p help -m cat-file > > If you try this yourself, you probably need "reset" or "stty sane" to > recover. > > Does this series address it?
No, it doesn’t.
The bug seems to be timing-related. The two copies of ‘less’ fight over initialization of the terminal and nobody wins.
What is the right thing to do in this case? Can we assume ‘man’ comes with a pager and suppress our own? Should we ask ‘man’ to suppress its pager, losing the formatting and status line in the process, but setting MANPAGER=cat? Probably the right thing to do would be to suppress our pager and set MANPAGER according to git_pager() so ‘man’ can use it.
Maybe something roughly like this (on top of the series)? This does not help backends that do not know how to use MANPAGER.
diff --git a/builtin/help.c b/builtin/help.c index 3182a2b..d9a0307 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -316,6 +316,40 @@ static const char *cmd_to_page(const char *git_cmd) return prepend("git", git_cmd); } +static void commit_pager_choice(void) { + switch (startup_info->use_pager) { + case 0: + setenv("GIT_PAGER", "cat", 1); + break; + case 1: + setup_pager(); + break; + default: + break; + } +} + +static void setup_man_pager(void) { + const char *pager; + + switch (startup_info->use_pager) { + case 0: + setenv("MANPAGER", "cat", 1); + break; + case 1: + pager = git_pager(isatty(1)); + if (!pager) { + setenv("MANPAGER", "cat", 1); + } else { + setenv("LESS", "FRSX", 0); + setenv("MANPAGER", pager, 1); + } + break; + default: + break; + } +} + static void setup_man_path(void) { struct strbuf new_path = STRBUF_INIT; @@ -358,6 +392,7 @@ static void show_man_page(const char *git_cmd) const char *fallback = getenv("GIT_MAN_VIEWER"); setup_man_path(); + setup_man_pager(); for (viewer = man_viewer_list; viewer; viewer = viewer->next) { exec_viewer(viewer->name, page); /* will return when unable */ @@ -423,20 +458,17 @@ int cmd_help(int argc, const char **argv, const char *prefix) builtin_help_usage, 0); parsed_help_format = help_format; - if (show_all) { + if (show_all || !argv[0]) { + commit_pager_choice(); printf("usage: %s\n\n", git_usage_string); - list_commands("git commands", &main_cmds, &other_cmds); + if (show_all) + list_commands("git commands", &main_cmds, &other_cmds); + else + list_common_cmds_help(); printf("%s\n", git_more_info_string); return 0; } - if (!argv[0]) { - printf("usage: %s\n\n", git_usage_string); - list_common_cmds_help(); - printf("\n%s\n", git_more_info_string); - return 0; - } - setup_git_directory_gently(&nongit); git_config(git_help_config, NULL); diff --git a/cache.h b/cache.h index 030ba01..8ba5727 100644 --- a/cache.h +++ b/cache.h @@ -1061,6 +1061,7 @@ int split_cmdline(char *cmdline, const char ***argv); /* git.c */ struct startup_info { int have_repository; + int use_pager; }; extern struct startup_info *startup_info; diff --git a/git.c b/git.c index 0a0d9eb..a81726e 100644 --- a/git.c +++ b/git.c @@ -14,7 +14,6 @@ const char git_more_info_string[] = "See 'git help COMMAND' for more information on a specific command."; static struct startup_info git_startup_info; -static int use_pager = -1; struct pager_config { const char *cmd; int val; @@ -39,7 +38,7 @@ int check_pager_config(const char *cmd) } static void commit_pager_choice(void) { - switch (use_pager) { + switch (startup_info->use_pager) { case 0: setenv("GIT_PAGER", "cat", 1); break; @@ -86,9 +85,9 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) puts(system_path(GIT_HTML_PATH)); exit(0); } else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) { - use_pager = 1; + startup_info->use_pager = 1; } else if (!strcmp(cmd, "--no-pager")) { - use_pager = 0; + startup_info->use_pager = 0; if (envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--no-replace-objects")) { @@ -221,13 +220,15 @@ static int handle_alias(int *argcp, const char ***argv) const char git_version_string[] = GIT_VERSION; #define RUN_SETUP (1<<0) -#define USE_PAGER (1<<1) /* * require working tree to be present -- anything uses this needs * RUN_SETUP for reading from the configuration file. */ -#define NEED_WORK_TREE (1<<2) -#define RUN_SETUP_GENTLY (1<<3) +#define NEED_WORK_TREE (1<<1) +#define RUN_SETUP_GENTLY (1<<2) +#define USE_PAGER (1<<3) +/* do not commit_pager_choice() -- the builtin will take care of it */ +#define NO_PAGER_SETUP (1<<4) struct cmd_struct { const char *cmd; @@ -244,6 +245,8 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) prefix = NULL; help = argc == 2 && !strcmp(argv[1], "-h"); if (!help) { + int use_pager = startup_info->use_pager; + if (p->option & RUN_SETUP) prefix = setup_git_directory(); if (p->option & RUN_SETUP_GENTLY) { @@ -255,8 +258,10 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) use_pager = check_pager_config(p->cmd); if (use_pager == -1 && p->option & USE_PAGER) use_pager = 1; + startup_info->use_pager = use_pager; } - commit_pager_choice(); + if (!(p->option & NO_PAGER_SETUP)) + commit_pager_choice(); if (!help && p->option & NEED_WORK_TREE) setup_work_tree(); @@ -328,7 +333,7 @@ static void handle_internal_command(int argc, const char **argv) { "get-tar-commit-id", cmd_get_tar_commit_id }, { "grep", cmd_grep, USE_PAGER }, { "hash-object", cmd_hash_object }, - { "help", cmd_help }, + { "help", cmd_help, NO_PAGER_SETUP }, { "index-pack", cmd_index_pack }, { "init", cmd_init_db }, { "init-db", cmd_init_db }, @@ -484,6 +489,7 @@ int main(int argc, const char **argv) const char *cmd; memset(&git_startup_info, 0, sizeof(git_startup_info)); + git_startup_info.use_pager = -1; startup_info = &git_startup_info; cmd = git_extract_argv0_path(argv[0]);