Re: [PATCH] Show html help with git-help --html

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] Show html help with git-help --html

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:14

"Nguyen Thai Ngoc Duy" [off-list ref] writes:
On 6/4/07, Junio C Hamano [off-list ref] wrote:
quoted
quoted
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3d8f03d..2ec8545 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -261,6 +261,18 @@ core.excludeFile::
...
+core.htmlprogram::
+     Specify the program used to open html help files when 'git-help'
+     is called with option --html or core.help is other than 'man'.
+     By default, xdg-open will be used.
Is the program's calling convention something that needs to be
customizable for this to be useful?
At first I thought xdg-open would be flexible enough for most Linux
systems because it will choose the best browser you have. But I now
recall that Git does not only run on Linux.  Will make it a parameter
in config.mak.in
Actually that was not what I meant.  I thought we might need to
support something like:

	core.htmlprogram = grand-unified-help %b

for a program that knows where in the system the distribution
keeps help HTML files, and add prefix (/usr/share/html-help/,
perhaps) and suffix (obviously, .html) to the basename of the
command (e.g. "grand-unified-help git-rebase" ends up running
"lynx file:///usr/share/html-help/git-rebase.html"), or perhaps

	core.htmlprogram = firefox /usr/share/html-help/%f

and we substitute '%f' with "git-rebase.html".
quoted
...
quoted
+     if (prefixcmp(git_cmd, "git"))
+             strcat(p,"git-");
+     strcat(p,git_cmd);
+     strcat(p,".html");
+
+     ret = system(p);
This is sloppy in the presense of potentially unsafe characters...
I personally think users will not shoot themselves with "git help
--html ';rm -rf'" but again scripts can. Thank you for pointing out.
Will add check for file existence before calling system().
File existence?  I am not sure what you mean by that, sorry.

Re: [PATCH] Show html help with git-help --html

From: Nguyen Thai Ngoc Duy <hidden>
Date: 2016-06-15 22:43:14

On 6/4/07, Junio C Hamano [off-list ref] wrote:
"Nguyen Thai Ngoc Duy" [off-list ref] writes:
quoted
On 6/4/07, Junio C Hamano [off-list ref] wrote:
quoted
quoted
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3d8f03d..2ec8545 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -261,6 +261,18 @@ core.excludeFile::
...
+core.htmlprogram::
+     Specify the program used to open html help files when 'git-help'
+     is called with option --html or core.help is other than 'man'.
+     By default, xdg-open will be used.
Is the program's calling convention something that needs to be
customizable for this to be useful?
At first I thought xdg-open would be flexible enough for most Linux
systems because it will choose the best browser you have. But I now
recall that Git does not only run on Linux.  Will make it a parameter
in config.mak.in
Actually that was not what I meant.  I thought we might need to
support something like:

        core.htmlprogram = grand-unified-help %b

for a program that knows where in the system the distribution
keeps help HTML files, and add prefix (/usr/share/html-help/,
perhaps) and suffix (obviously, .html) to the basename of the
command (e.g. "grand-unified-help git-rebase" ends up running
"lynx file:///usr/share/html-help/git-rebase.html"), or perhaps

        core.htmlprogram = firefox /usr/share/html-help/%f

and we substitute '%f' with "git-rebase.html".
Ok. Now I get it.
quoted
quoted
...
quoted
+     if (prefixcmp(git_cmd, "git"))
+             strcat(p,"git-");
+     strcat(p,git_cmd);
+     strcat(p,".html");
+
+     ret = system(p);
This is sloppy in the presense of potentially unsafe characters...
I personally think users will not shoot themselves with "git help
--html ';rm -rf'" but again scripts can. Thank you for pointing out.
Will add check for file existence before calling system().
File existence?  I am not sure what you mean by that, sorry.
Something like this on top of the last patch.
diff --git a/help.c b/help.c
index e3e705b..6f5c340 100644
--- a/help.c
+++ b/help.c
@@ -191,7 +191,8 @@ static void show_html_page(const char *git_cmd)
 {
        const char *html_dir;
        int i,len,ret;
-       char *p;
+       char *p,*pathname;
+       struct stat html_stat;

        html_dir = HTML_DIR;
        if (!html_help_program)
@@ -203,13 +204,19 @@ static void show_html_page(const char *git_cmd)

        strcpy(p, html_help_program);
        strcat(p," ");
+       pathname = p + strlen(p);
        strcat(p,html_dir);
        if (prefixcmp(git_cmd, "git"))
                strcat(p,"git-");
        strcat(p,git_cmd);
        strcat(p,".html");

-       ret = system(p);
+       if (!stat(pathname, &html_stat))
+               ret = system(p);
+       else {
+               error("%s not found",pathname);
+               ret = -1;
+       }

        /* fallback to man pages */
        if (show_html_help > 1 && (ret == -1 || ret > 0))
-- 
Duy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help