Junio C Hamano wrote:
> 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]);
--
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