{"thread":{"id":"12352","subject":"[PATCH 1/2] help: add \"man.viewer\" config var to use \"woman\" or \"konqueror\"","startedAt":"2008-02-28T04:19:56Z","lastAt":"2008-03-03T19:07:49Z","messageCount":12,"participants":["Christian Couder","Xavier Maillard","Junio C Hamano","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"70207","messageId":"20080228051956.2f36edf0.chriscool@tuxfamily.org","threadId":"12352","inReplyTo":null,"subject":"[PATCH 1/2] help: add \"man.viewer\" config var to use \"woman\" or \"konqueror\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-02-28T04:19:56Z","receivedAt":"2008-02-28T04:19:56Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This patch makes it possible to view man pages using other tools\nthan the \"man\" program. It also implements support for emacs'\n\"woman\" and konqueror with the man KIO slave to view man pages.\n\nNote that \"emacsclient\" is used with option \"-e\" to launch \"woman\"\non emacs and this works only on versions >= 22.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n help.c |   81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 files changed, 80 insertions(+), 1 deletions(-)\n\n\tA few minor changes since the previous version:\n\t- remove an unused \"size\" variable,\n\t- use right style for multi line comments,\n\t- use (void) for function with no arguments,\n\t- use [PATCH ...] in email subject.\n\n\tIt should apply on top of 'next'.\n\ndiff --git a/help.c b/help.c\nindex e57a50e..2cb152d 100644\n--- a/help.c\n+++ b/help.c\n@@ -8,6 +8,9 @@\n #include \"exec_cmd.h\"\n #include \"common-cmds.h\"\n #include \"parse-options.h\"\n+#include \"run-command.h\"\n+\n+static const char *man_viewer;\n \n enum help_format {\n \tHELP_FORMAT_MAN,\n@@ -50,6 +53,8 @@ static int git_help_config(const char *var, const char *value)\n \t\thelp_format = parse_help_format(value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"man.viewer\"))\n+\t\treturn git_config_string(&man_viewer, var, value);\n \treturn git_default_config(var, value);\n }\n \n@@ -345,11 +350,85 @@ static void setup_man_path(void)\n \tstrbuf_release(&new_path);\n }\n \n+static int check_emacsclient_version(void)\n+{\n+\tstruct strbuf buffer = STRBUF_INIT;\n+\tstruct child_process ec_process;\n+\tconst char *argv_ec[] = { \"emacsclient\", \"--version\", NULL };\n+\tint version;\n+\n+\t/* emacsclient prints its version number on stderr */\n+\tmemset(&ec_process, 0, sizeof(ec_process));\n+\tec_process.argv = argv_ec;\n+\tec_process.err = -1;\n+\tec_process.stdout_to_stderr = 1;\n+\tif (start_command(&ec_process)) {\n+\t\tfprintf(stderr, \"Failed to start emacsclient.\\n\");\n+\t\treturn -1;\n+\t}\n+\tstrbuf_read(&buffer, ec_process.err, 20);\n+\tclose(ec_process.err);\n+\n+\t/*\n+\t * Don't bother checking return value, because \"emacsclient --version\"\n+\t * seems to always exits with code 1.\n+\t */\n+\tfinish_command(&ec_process);\n+\n+\tif (prefixcmp(buffer.buf, \"emacsclient\")) {\n+\t\tfprintf(stderr, \"Failed to parse emacsclient version.\\n\");\n+\t\tstrbuf_release(&buffer);\n+\t\treturn -1;\n+\t}\n+\n+\tstrbuf_remove(&buffer, 0, strlen(\"emacsclient\"));\n+\tversion = atoi(buffer.buf);\n+\n+\tif (version < 22) {\n+\t\tfprintf(stderr,\n+\t\t\t\"emacsclient version '%d' too old (< 22).\\n\",\n+\t\t\tversion);\n+\t\tstrbuf_release(&buffer);\n+\t\treturn -1;\n+\t}\n+\n+\tstrbuf_release(&buffer);\n+\treturn 0;\n+}\n+\n+static void exec_woman_emacs(const char *page)\n+{\n+\tif (!check_emacsclient_version()) {\n+\t\t/* This works only with emacsclient version >= 22. */\n+\t\tstruct strbuf man_page = STRBUF_INIT;\n+\t\tstrbuf_addf(&man_page, \"(woman \\\"%s\\\")\", page);\n+\t\texeclp(\"emacsclient\", \"emacsclient\", \"-e\", man_page.buf, NULL);\n+\t} else\n+\t\texeclp(\"man\", \"man\", page, NULL);\n+}\n+\n+static void exec_man_konqueror(const char *page)\n+{\n+\tconst char *display = getenv(\"DISPLAY\");\n+\tif (display && *display) {\n+\t\tstruct strbuf man_page = STRBUF_INIT;\n+\t\tstrbuf_addf(&man_page, \"man:%s(1)\", page);\n+\t\texeclp(\"kfmclient\", \"kfmclient\", \"newTab\", man_page.buf, NULL);\n+\t} else\n+\t\texeclp(\"man\", \"man\", page, NULL);\n+}\n+\n static void show_man_page(const char *git_cmd)\n {\n \tconst char *page = cmd_to_page(git_cmd);\n \tsetup_man_path();\n-\texeclp(\"man\", \"man\", page, NULL);\n+\tif (!man_viewer || !strcmp(man_viewer, \"man\"))\n+\t\texeclp(\"man\", \"man\", page, NULL);\n+\tif (!strcmp(man_viewer, \"woman\"))\n+\t\texec_woman_emacs(page);\n+\tif (!strcmp(man_viewer, \"konqueror\"))\n+\t\texec_man_konqueror(page);\n+\tdie(\"'%s': unsupported man viewer.\", man_viewer);\n }\n \n static void show_info_page(const char *git_cmd)\n-- \n1.5.4.3.328.gcaed.dirty\n"},{"id":"70357","messageId":"200802290200.m1T2023x017992@localhost.localdomain","threadId":"12352","inReplyTo":"20080228051956.2f36edf0.chriscool@tuxfamily.org","subject":"Re: [PATCH 1/2] help: add \"man.viewer\" config var to use \"woman\" or \"konqueror\"","fromName":"Xavier Maillard","fromEmail":"xma@gnu.org","sentAt":"2008-02-29T02:00:02Z","receivedAt":"2008-02-29T02:00:02Z","isPatch":true,"sender":{"key":"xma@gnu.org","avatar":null},"body":"Hi,\n\n   Note that \"emacsclient\" is used with option \"-e\" to launch \"woman\"\n   on emacs and this works only on versions >= 22.\n\nI tested and it did not want to work at first. I modified it a\nlittle bit and now it works with GNU Emacs 23.0.60 (CVS TRUNK).\nDunno if emacsclient has changed between 22 and CVS but it seems\nit is printing onto stdout now. I will have to check that.\n\nI am just attaching the patch with my modifications. Please\ncomment it and feel free to adapt it with yours.\n\nNotice the if(!&buffer) is there just for me. It could be removed\nsafely. Also note that I did not develop C for ages so apologize\nfor my approximations ;)\n\n\ndiff --git a/help.c b/help.c\nindex e57a50e..b939f8d 100644\n--- a/help.c\n+++ b/help.c\n@@ -8,6 +8,9 @@\n #include \"exec_cmd.h\"\n #include \"common-cmds.h\"\n #include \"parse-options.h\"\n+#include \"run-command.h\"\n+\n+static const char *man_viewer;\n \n enum help_format {\n \tHELP_FORMAT_MAN,\n@@ -50,6 +53,8 @@ static int git_help_config(const char *var, const char *value)\n \t\thelp_format = parse_help_format(value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"man.viewer\"))\n+\t\treturn git_config_string(&man_viewer, var, value);\n \treturn git_default_config(var, value);\n }\n \n@@ -345,11 +350,90 @@ static void setup_man_path(void)\n \tstrbuf_release(&new_path);\n }\n \n+static int check_emacsclient_version(void)\n+{\n+\tstruct strbuf buffer = STRBUF_INIT;\n+\tstruct child_process ec_process;\n+\n+\tconst char *argv_ec[] = { \"emacsclient\", \"--version\", NULL };\n+\tint version;\n+\tsize_t len;\n+\n+\t/* emacsclient prints its version number on stderr */\n+\tmemset(&ec_process, 0, sizeof(ec_process));\n+\tec_process.argv = argv_ec;\n+\tec_process.err = -1;\n+\tec_process.out = -1;\n+\tec_process.stdout_to_stderr = 0;\n+\n+\tif (start_command(&ec_process))\n+\t\treturn error(\"Failed to start emacsclient.\");\n+\n+\tlen = strbuf_read(&buffer, ec_process.out, 20);\n+\tclose(ec_process.out);\n+\n+\t/*\n+\t * Don't bother checking return value, because \"emacsclient --version\"\n+\t * seems to always exits with code 1.\n+\t */\n+\tfinish_command(&ec_process);\n+\n+\tif (!&buffer)\n+\t\treturn;\n+\n+\tif (!len || len < 0 || prefixcmp(buffer.buf, \"emacsclient\")) {\n+\t\tstrbuf_release(&buffer);\n+\t\treturn error(\"Failed to parse emacsclient version.\");\n+\t}\n+\n+\tstrbuf_remove(&buffer, 0, strlen(\"emacsclient\"));\n+\tversion = atoi(buffer.buf);\n+\n+\tif (version < 22) {\n+\t\tfprintf(stderr,\n+\t\t\t\"emacsclient version '%d' too old (< 22).\\n\",\n+\t\t\tversion);\n+\t\tstrbuf_release(&buffer);\n+\t\treturn -1;\n+\t}\n+\n+\tstrbuf_release(&buffer);\n+\treturn 0;\n+}\n+\n+static void exec_woman_emacs(const char *page)\n+{\n+\tif (!check_emacsclient_version()) {\n+\t\t/* This works only with emacsclient version >= 22. */\n+\t\tstruct strbuf man_page = STRBUF_INIT;\n+\t\tstrbuf_addf(&man_page, \"(woman \\\"%s\\\")\", page);\n+\t\texeclp(\"emacsclient\", \"emacsclient\", \"-e\", man_page.buf, NULL);\n+\t} else\n+\t\texeclp(\"man\", \"man\", page, NULL);\n+}\n+\n+static void exec_man_konqueror(const char *page)\n+{\n+\tconst char *display = getenv(\"DISPLAY\");\n+\tif (display && *display) {\n+\t\tstruct strbuf man_page = STRBUF_INIT;\n+\t\tstrbuf_addf(&man_page, \"man:%s(1)\", page);\n+\t\texeclp(\"kfmclient\", \"kfmclient\", \"newTab\", man_page.buf, NULL);\n+\t} else\n+\t\texeclp(\"man\", \"man\", page, NULL);\n+}\n+\n static void show_man_page(const char *git_cmd)\n {\n \tconst char *page = cmd_to_page(git_cmd);\n \tsetup_man_path();\n-\texeclp(\"man\", \"man\", page, NULL);\n+\tif (!man_viewer || !strcmp(man_viewer, \"man\"))\n+\t\texeclp(\"man\", \"man\", page, NULL);\n+\tif (!strcmp(man_viewer, \"woman\"))\n+\t\texec_woman_emacs(page);\n+\tif (!strcmp(man_viewer, \"konqueror\"))\n+\t\texec_man_konqueror(page);\n+\tdie(\"'%s': unsupported man viewer.\", man_viewer);\n }\n \n static void show_info_page(const char *git_cmd)\n\n\n\n\n\tXavier\n-- \nhttp://www.gnu.org\nhttp://www.april.org\nhttp://www.lolica.org\n"},{"id":"70359","messageId":"200802290200.m1T20I7Z018030@localhost.localdomain","threadId":"12352","inReplyTo":"20080228051956.2f36edf0.chriscool@tuxfamily.org","subject":"Re: [PATCH 1/2] help: add \"man.viewer\" config var to use \"woman\" or \"konqueror\"","fromName":"Xavier Maillard","fromEmail":"xma@gnu.org","sentAt":"2008-02-29T02:00:18Z","receivedAt":"2008-02-29T02:00:18Z","isPatch":true,"sender":{"key":"xma@gnu.org","avatar":null},"body":"Hi (again),\n\n   Note that \"emacsclient\" is used with option \"-e\" to launch \"woman\"\n   on emacs and this works only on versions >= 22.\n\nHere are more clues concerning emacsclient I found out when\ntrying to understand why it worked for you with >=22 and not for\nme with a 23.x version.\n\n   +\n   +\t/* emacsclient prints its version number on stderr */\n\nAs commented into another post, here the version is displayed on\nstdout which is confirmed by reading the source code:\n\n/* Display a normal or error message.\n   On Windows, use a message box if compiled as a Windows app.  */\n\n      FILE *f = is_error ? stderr : stdout;\n      fputs (msg, f);\n      fflush (f);\n\nis_error is a boolean parameter to the function message.\n\n   +\t/*\n   +\t * Don't bother checking return value, because \"emacsclient --version\"\n   +\t * seems to always exits with code 1.\n   +\t */\n\nAlmost, in fact, in emacsclient.c, we can read\n\n\tcase 'V':\n\t  message (FALSE, \"emacsclient %s\\n\", VERSION);\n\t  exit (EXIT_SUCCESS);\n\t  break;\n\nSo it always returns 0.\n\nRegards\n\n\tXavier\n-- \nhttp://www.gnu.org\nhttp://www.april.org\nhttp://www.lolica.org\n"},{"id":"70363","messageId":"200802290814.39366.chriscool@tuxfamily.org","threadId":"12352","inReplyTo":"200802290200.m1T2023x017992@localhost.localdomain","subject":"Re: [PATCH 1/2] help: add \"man.viewer\" config var to use \"woman\" or \"konqueror\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-02-29T07:14:39Z","receivedAt":"2008-02-29T07:14:39Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nLe vendredi 29 février 2008, Xavier Maillard a écrit :\n> Hi,\n>\n>    Note that \"emacsclient\" is used with option \"-e\" to launch \"woman\"\n>    on emacs and this works only on versions >= 22.\n>\n> I tested and it did not want to work at first. I modified it a\n> little bit and now it works with GNU Emacs 23.0.60 (CVS TRUNK).\n> Dunno if emacsclient has changed between 22 and CVS but it seems\n> it is printing onto stdout now. I will have to check that.\n\nIn fact on my main development machine there is only Emacsclient 21 and it \noutputs its version on stderr and exits with code 1. Emacsclient 21 also \ndoes not accept \"-e\" option.\n\nI just checked with Emacsclient 22 and it outputs its version on stdout and \nexits with code 0. But it accepts the \"-e\" option.\n\nI will need to get Emacs 23 from CVS but from what you say I guess it works \nlike Emacs 22. \n\nSo my \"check_emacsclient_version\" function works only with emacsclient 21 \nand may be earlier versions.\n\nI used:\n\n\t/* emacsclient prints its version number on stderr */\n\tmemset(&ec_process, 0, sizeof(ec_process));\n\tec_process.argv = argv_ec;\n\tec_process.err = -1;\n\tec_process.stdout_to_stderr = 1;\n\nI hoped the above line would redirect stdout to stderr and I could read both \nfrom \"ec_process.err\" below, but it seems it doesn't work like that. I also \njust checked using \"ec_process.stdout_to_stderr = 0;\" but it still doesn't \nwork. \n\n\tif (start_command(&ec_process)) {\n\t\tfprintf(stderr, \"Failed to start emacsclient.\\n\");\n\t\treturn -1;\n\t}\n\tstrbuf_read(&buffer, ec_process.err, 20);\n\tclose(ec_process.err);\n\nI will be offline this week end but I hope I can debug this next week.\n\nThanks,\nChristian.\n"},{"id":"70365","messageId":"7vfxvcdx23.fsf@gitster.siamese.dyndns.org","threadId":"12352","inReplyTo":"200802290814.39366.chriscool@tuxfamily.org","subject":"Re: [PATCH 1/2] help: add \"man.viewer\" config var to use \"woman\" or \"konqueror\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-29T07:46:44Z","receivedAt":"2008-02-29T07:46:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Looking at this exchange really makes me suspect that these\nthings should not be in C.  Every time Emacs updates we need\nrecompilation?  Yuck.\n\nWhat was the reason we gave up calling out to a single generic\nscripted wrapper that the users can just munge to suit their\ntastes?\n"},{"id":"70453","messageId":"200803010100.m21105LU008843@localhost.localdomain","threadId":"12352","inReplyTo":"7vfxvcdx23.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2] help: add \"man.viewer\" config var to use \"woman\" or \"konqueror\"","fromName":"Xavier Maillard","fromEmail":"xma@gnu.org","sentAt":"2008-03-01T01:00:05Z","receivedAt":"2008-03-01T01:00:05Z","isPatch":true,"sender":{"key":"xma@gnu.org","avatar":null},"body":"\n   Looking at this exchange really makes me suspect that these\n   things should not be in C.  Every time Emacs updates we need\n   recompilation?  Yuck.\n\nIn fact, I am pretty confident that emacsclient has always\nprinted its version onto stdout so I do not think we need to\nrecompile every week :)\n\n   What was the reason we gave up calling out to a single generic\n   scripted wrapper that the users can just munge to suit their\n   tastes?\n\nGood question. Though I find chriscool's implementation nicely\nthought. With the modifications I made, this works as expected\nwith either emacs22 and emacs23 (aka CVS). What's more, how a\nuser could have tested if emacsclient was provided in the right\nversion using a generic scripted wrapper ?\n\nI guess, chriscool should provide a more consistent emacsclient\nwrapper with support for GNU Emacs version >= 21. This won't be\nan enormous amount of work to do it (I can help him if he wants\nto but I do not want to steal the paternity of his idea).\n\n\tXavier\n-- \nhttp://www.gnu.org\nhttp://www.april.org\nhttp://www.lolica.org\n"},{"id":"70455","messageId":"200803010100.m2110ULs008889@localhost.localdomain","threadId":"12352","inReplyTo":"200802290814.39366.chriscool@tuxfamily.org","subject":"[PATCH AMENDED] help: add \"man.viewer\" config var to use \"woman\" or \"konqueror\"","fromName":"Xavier Maillard","fromEmail":"xma@gnu.org","sentAt":"2008-03-01T01:00:31Z","receivedAt":"2008-03-01T01:00:31Z","isPatch":true,"sender":{"key":"xma@gnu.org","avatar":null},"body":"\nThis patch makes it possible to view man pages using other tools\nthan the \"man\" program. It also implements support for emacs'\n\"woman\" and konqueror with the man KIO slave to view man pages.\n\nNote that \"emacsclient\" is used with option \"-e\" to launch \"woman\"\non GNU emacs and this works only on versions >= 22.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\nAmended-by: Xavier Maillard <xma@gnu.org>\n---\nI have tested it with emacsclient 22.x and 23.x. Previous\nversions won't work at the moment.\n\n help.c |   85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 files changed, 84 insertions(+), 1 deletions(-)\n\ndiff --git a/help.c b/help.c\nindex e57a50e..eb138f0 100644\n--- a/help.c\n+++ b/help.c\n@@ -8,6 +8,9 @@\n #include \"exec_cmd.h\"\n #include \"common-cmds.h\"\n #include \"parse-options.h\"\n+#include \"run-command.h\"\n+\n+static const char *man_viewer;\n \n enum help_format {\n \tHELP_FORMAT_MAN,\n@@ -50,6 +53,8 @@ static int git_help_config(const char *var, const char *value)\n \t\thelp_format = parse_help_format(value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"man.viewer\"))\n+\t\treturn git_config_string(&man_viewer, var, value);\n \treturn git_default_config(var, value);\n }\n \n@@ -345,11 +350,89 @@ static void setup_man_path(void)\n \tstrbuf_release(&new_path);\n }\n \n+static int check_emacsclient_version(void)\n+{\n+\tstruct strbuf buffer = STRBUF_INIT;\n+\tstruct child_process ec_process;\n+\n+\tconst char *argv_ec[] = { \"emacsclient\", \"--version\", NULL };\n+\tint version;\n+\tsize_t len;\n+\n+\t/* emacsclient prints its version number on stdout */\n+\tmemset(&ec_process, 0, sizeof(ec_process));\n+\tec_process.argv = argv_ec;\n+\tec_process.out = -1;\n+\tec_process.stdout_to_stderr = 0;\n+\n+\tif (start_command(&ec_process))\n+\t\treturn error(\"Failed to start emacsclient.\");\n+\n+\tlen = strbuf_read(&buffer, ec_process.out, 20);\n+\tclose(ec_process.out);\n+\n+\t/*\n+\t * Don't bother checking return value, because \"emacsclient --version\"\n+\t * always exits with code 0.\n+\t */\n+\tfinish_command(&ec_process);\n+\n+\tif (!&buffer)\n+\t\treturn -1;\n+\n+\tif (!len || len < 0 || prefixcmp(buffer.buf, \"emacsclient\")) {\n+\t\tstrbuf_release(&buffer);\n+\t\treturn error(\"Failed to parse emacsclient version.\");\n+\t}\n+\n+\tstrbuf_remove(&buffer, 0, strlen(\"emacsclient\"));\n+\tversion = atoi(buffer.buf);\n+\n+\tif (version < 22) {\n+\t\tfprintf(stderr,\n+\t\t\t\"emacsclient version '%d' too old (< 22).\\n\",\n+\t\t\tversion);\n+\t\tstrbuf_release(&buffer);\n+\t\treturn -1;\n+\t}\n+\n+\tstrbuf_release(&buffer);\n+\treturn 0;\n+}\n+\n+static void exec_woman_emacs(const char *page)\n+{\n+\tif (!check_emacsclient_version()) {\n+\t\t/* This works only with emacsclient version >= 22. */\n+\t\tstruct strbuf man_page = STRBUF_INIT;\n+\t\tstrbuf_addf(&man_page, \"(woman \\\"%s\\\")\", page);\n+\t\texeclp(\"emacsclient\", \"emacsclient\", \"-e\", man_page.buf, NULL);\n+\t} else // revert to old good man program\n+\t\texeclp(\"man\", \"man\", page, NULL);\n+}\n+\n+static void exec_man_konqueror(const char *page)\n+{\n+\tconst char *display = getenv(\"DISPLAY\");\n+\tif (display && *display) {\n+\t\tstruct strbuf man_page = STRBUF_INIT;\n+\t\tstrbuf_addf(&man_page, \"man:%s(1)\", page);\n+\t\texeclp(\"kfmclient\", \"kfmclient\", \"newTab\", man_page.buf, NULL);\n+\t} else\n+\t\texeclp(\"man\", \"man\", page, NULL);\n+}\n+\n static void show_man_page(const char *git_cmd)\n {\n \tconst char *page = cmd_to_page(git_cmd);\n \tsetup_man_path();\n-\texeclp(\"man\", \"man\", page, NULL);\n+\tif (!man_viewer || !strcmp(man_viewer, \"man\"))\n+\t\texeclp(\"man\", \"man\", page, NULL);\n+\tif (!strcmp(man_viewer, \"woman\"))\n+\t\texec_woman_emacs(page);\n+\tif (!strcmp(man_viewer, \"konqueror\"))\n+\t\texec_man_konqueror(page);\n+\tdie(\"'%s': unsupported man viewer.\", man_viewer);\n }\n \n static void show_info_page(const char *git_cmd)\n-- \n1.5.4.3.343.gc696\n"},{"id":"70458","messageId":"7vir079ong.fsf@gitster.siamese.dyndns.org","threadId":"12352","inReplyTo":"200803010100.m2110ULs008889@localhost.localdomain","subject":"Re: [PATCH AMENDED] help: add \"man.viewer\" config var to use \"woman\" or \"konqueror\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-01T02:14:11Z","receivedAt":"2008-03-01T02:14:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Xavier Maillard <xma@gnu.org> writes:\n\n> This patch makes it possible to view man pages using other tools\n> than the \"man\" program. It also implements support for emacs'\n> \"woman\" and konqueror with the man KIO slave to view man pages.\n>\n> Note that \"emacsclient\" is used with option \"-e\" to launch \"woman\"\n> on GNU emacs and this works only on versions >= 22.\n>\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> Amended-by: Xavier Maillard <xma@gnu.org>\n\nWhen you do something like this, please CC the original author\nto keep him in the loop.\n\n> +static void exec_woman_emacs(const char *page)\n> +{\n> +\tif (!check_emacsclient_version()) {\n> +\t\t/* This works only with emacsclient version >= 22. */\n> +\t\tstruct strbuf man_page = STRBUF_INIT;\n> +\t\tstrbuf_addf(&man_page, \"(woman \\\"%s\\\")\", page);\n> +\t\texeclp(\"emacsclient\", \"emacsclient\", \"-e\", man_page.buf, NULL);\n> +\t} else // revert to old good man program\n> +\t\texeclp(\"man\", \"man\", page, NULL);\n> +}\n> +\n> +static void exec_man_konqueror(const char *page)\n> +{\n> +\tconst char *display = getenv(\"DISPLAY\");\n> +\tif (display && *display) {\n> +\t\tstruct strbuf man_page = STRBUF_INIT;\n> +\t\tstrbuf_addf(&man_page, \"man:%s(1)\", page);\n> +\t\texeclp(\"kfmclient\", \"kfmclient\", \"newTab\", man_page.buf, NULL);\n> +\t} else\n> +\t\texeclp(\"man\", \"man\", page, NULL);\n> +}\n> +\n\nI really do not think doing it this way would scale.  What if\nsomebody prefers konq under X, and otherwise woman but wants to\nfall back to man?\n\nI do not think checking emacsclient version _every time_ is a\ngood idea either, but for the sake of discussing an alternative\napproach, let's say that is fine.\n\nHow about allowing multi-valued man.viewer like this:\n\n        [man]\n                viewer = woman\n                viewer = konqueror\n                viewer = man\n\nand have:\n\n        static struct man_viewer {\n                char *name;\n                void (*exec)(const char *);\n        } viewers[] = {\n                { \"woman\", exec_woman },\n                { \"konqueror\", exec_konqueror },\n                { \"man\", exec_man },\n                { NULL, },\n        };\n\nThen you can iterate the man.viewer values, ask the viewer's\nexec() function to show the page (or return when it is not\nin an environment that it can be useful).\n\nshow_man_page() would become:\n\n\tfor (each viewer in user's config)\n\t\tviewer.exec(page); /* will return when unable */\n\tdie(\"no man viewer handled the request\");\n\n"},{"id":"70754","messageId":"200803030820.52842.chriscool@tuxfamily.org","threadId":"12352","inReplyTo":"200803010100.m21105LU008843@localhost.localdomain","subject":"Re: [PATCH 1/2] help: add \"man.viewer\" config var to use \"woman\" or \"konqueror\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-03-03T07:20:52Z","receivedAt":"2008-03-03T07:20:52Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le samedi 1 mars 2008, Xavier Maillard a écrit :\n>    Looking at this exchange really makes me suspect that these\n>    things should not be in C.  Every time Emacs updates we need\n>    recompilation?  Yuck.\n>\n> In fact, I am pretty confident that emacsclient has always\n> printed its version onto stdout so I do not think we need to\n> recompile every week :)\n\nOn my Kubuntu Feisty development machine with the stock emacs \n21.4a+1-2ubuntu1.1 package I get:\n\n$ emacsclient --version\nemacsclient 21.4\n$ emacsclient --version 2>/dev/null\n$ emacsclient --version 1>/dev/null\nemacsclient 21.4\n\nAnd I don't think it specific to Kubuntu because I remenber getting the same \nwith emacs21 (before installing emacs22) on another (Debian testing) \nmachine.\n\n>    What was the reason we gave up calling out to a single generic\n>    scripted wrapper that the users can just munge to suit their\n>    tastes?\n\nI didn't gave up, I am just waiting for the implementation of a scripted \nwrapper in git-mergetool.sh, that is now in 'pu', to mature. Then I will \nsteal it so we have the same for git help.\n\n> Good question. Though I find chriscool's implementation nicely\n> thought.\n\nThanks.\n\n> With the modifications I made, this works as expected \n> with either emacs22 and emacs23 (aka CVS). What's more, how a\n> user could have tested if emacsclient was provided in the right\n> version using a generic scripted wrapper ?\n\nI agree that it's better to do it for the users.\n\nIn fact just after this email, I will send a patch to \"run_command.c\" named:\n\n\"Redirect stderr to a pipe before redirecting stdout to stderr\"\n\nthat will make my last patches to \"help.c\" work with both Emacs21 and \nEmacs22.\n\n> I guess, chriscool should provide a more consistent emacsclient\n> wrapper with support for GNU Emacs version >= 21. This won't be\n> an enormous amount of work to do it (I can help him if he wants\n> to but I do not want to steal the paternity of his idea).\n\nIt would help if you could test with Emacs23 as I didn't installed it.\n\nThank you for testing my patches.\n\nChristian.\n"},{"id":"70757","messageId":"200803030838.48976.chriscool@tuxfamily.org","threadId":"12352","inReplyTo":"7vir079ong.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH AMENDED] help: add \"man.viewer\" config var to use \"woman\" or \"konqueror\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-03-03T07:38:48Z","receivedAt":"2008-03-03T07:38:48Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le samedi 1 mars 2008, Junio C Hamano a écrit :\n> Xavier Maillard <xma@gnu.org> writes:\n> > This patch makes it possible to view man pages using other tools\n> > than the \"man\" program. It also implements support for emacs'\n> > \"woman\" and konqueror with the man KIO slave to view man pages.\n> >\n> > Note that \"emacsclient\" is used with option \"-e\" to launch \"woman\"\n> > on GNU emacs and this works only on versions >= 22.\n> >\n> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> > Amended-by: Xavier Maillard <xma@gnu.org>\n>\n> When you do something like this, please CC the original author\n> to keep him in the loop.\n\nYes please.\n\n> I really do not think doing it this way would scale.  What if\n> somebody prefers konq under X, and otherwise woman but wants to\n> fall back to man?\n>\n> I do not think checking emacsclient version _every time_ is a\n> good idea either, but for the sake of discussing an alternative\n> approach, let's say that is fine.\n>\n> How about allowing multi-valued man.viewer like this:\n>\n>         [man]\n>                 viewer = woman\n>                 viewer = konqueror\n>                 viewer = man\n\nI tried the above in my .git/config and I get:\n\n$ git config man.viewer\nwoman\nerror: More than one value for the key man.viewer: konqueror\nerror: More than one value for the key man.viewer: man\n\nso I guess this can work only in C.\n\n> and have:\n>\n>         static struct man_viewer {\n>                 char *name;\n>                 void (*exec)(const char *);\n>         } viewers[] = {\n>                 { \"woman\", exec_woman },\n>                 { \"konqueror\", exec_konqueror },\n>                 { \"man\", exec_man },\n>                 { NULL, },\n>         };\n>\n> Then you can iterate the man.viewer values, ask the viewer's\n> exec() function to show the page (or return when it is not\n> in an environment that it can be useful).\n>\n> show_man_page() would become:\n>\n> \tfor (each viewer in user's config)\n> \t\tviewer.exec(page); /* will return when unable */\n> \tdie(\"no man viewer handled the request\");\n\nGreat idea, I will see if I can do that in some latter patches.\n\nThanks,\nChristian.\n"},{"id":"70759","messageId":"fqga4j$hgt$1@ger.gmane.org","threadId":"12352","inReplyTo":"200803030838.48976.chriscool@tuxfamily.org","subject":"Re: [PATCH AMENDED] help: add \"man.viewer\" config var to use \"woman\" or \"konqueror\"","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-03-03T07:42:23Z","receivedAt":"2008-03-03T07:42:23Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Christian Couder wrote:\n> Le samedi 1 mars 2008, Junio C Hamano a écrit :\n\n>> How about allowing multi-valued man.viewer like this:\n>>\n>>         [man]\n>>                 viewer = woman\n>>                 viewer = konqueror\n>>                 viewer = man\n> \n> I tried the above in my .git/config and I get:\n> \n> $ git config man.viewer\n> woman\n> error: More than one value for the key man.viewer: konqueror\n> error: More than one value for the key man.viewer: man\n> \n> so I guess this can work only in C.\n\n$ git config --get-all man.viewer \n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n\n\n"},{"id":"70827","messageId":"200803032007.49234.chriscool@tuxfamily.org","threadId":"12352","inReplyTo":"fqga4j$hgt$1@ger.gmane.org","subject":"Re: [PATCH AMENDED] help: add \"man.viewer\" config var to use \"woman\" or \"konqueror\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-03-03T19:07:49Z","receivedAt":"2008-03-03T19:07:49Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le lundi 3 mars 2008, Jakub Narebski a écrit :\n> Christian Couder wrote:\n> > Le samedi 1 mars 2008, Junio C Hamano a écrit :\n> >> How about allowing multi-valued man.viewer like this:\n> >>\n> >>         [man]\n> >>                 viewer = woman\n> >>                 viewer = konqueror\n> >>                 viewer = man\n> >\n> > I tried the above in my .git/config and I get:\n> >\n> > $ git config man.viewer\n> > woman\n> > error: More than one value for the key man.viewer: konqueror\n> > error: More than one value for the key man.viewer: man\n> >\n> > so I guess this can work only in C.\n>\n> $ git config --get-all man.viewer\n\nOoops, you are right, sorry about the noise.\n\nChristian.\n"}]}