From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Mon, 04 Jun 2007 13:29:22 GMT
Subject: Re: [PATCH] Show html help with git-help --html
Message-ID: <fcaeb9bf0706040629k489d4818sa291725cbbeead79@mail.gmail.com>
In-Reply-To: <7vps4cjg1w.fsf@assigned-by-dhcp.cox.net>

On 6/4/07, Junio C Hamano <gitster@pobox.com> wrote:
> > 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

>
> > diff --git a/Makefile b/Makefile
> > index cac0a4a..43e0d15 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -145,6 +145,7 @@ prefix = $(HOME)
> >  bindir = $(prefix)/bin
> >  gitexecdir = $(bindir)
> >  sharedir = $(prefix)/share/
> > +htmldir = $(sharedir)/html/
> >  template_dir = $(sharedir)/git-core/templates/
> >  ifeq ($(prefix),/usr)
> >  sysconfdir = /etc
>
> Is it customary to have HTMLized documentation material for
> different packages all together in a single .../share/html/
> directory, like manpages are placed in share/man/man1/
> directory?  I somehow had an impression that a layout to have
> html directory per package (i.e. share/doc/$pkg/html/) was more
> common.  I dunno.

The default value is not really useful. I would leave that for
distribution package mantainers to decide proper location because they
have to install html files separately anyway (at least in Gentoo).
However it's not convenient for compiling-from-source users. Will redo
the patch and add rules to install html files also.

>
> > diff --git a/help.c b/help.c
> > index 6a9af4d..e3e705b 100644
> > --- a/help.c
> > +++ b/help.c
> > @@ -183,6 +187,36 @@ static void show_man_page(const char *git_cmd)
> >       execlp("man", "man", page, NULL);
> >  }
> >
> > +static void show_html_page(const char *git_cmd)
> > +{
> > +     const char *html_dir;
> > +     int i,len,ret;
> > +     char *p;
> > +
> > +     html_dir = HTML_DIR;
> > +     if (!html_help_program)
> > +             html_help_program = "xdg-open";
> > +
> > +     /* html_help_program space html_dir git- git_cmd .html */
> > +     len = strlen(html_help_program) + 1 + strlen(html_dir) + 4 + strlen(git_cmd) + 5;
> > +     p = xmalloc(len + 1);
> > +
> > +     strcpy(p, html_help_program);
> > +     strcat(p," ");
> > +     strcat(p,html_dir);
> > +     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().
-- 
Duy