[PATCH 2/3] git-help: add -w|--web option to display html man page in a browser.

[PATCH 2/3] git-help: add -w|--web option to display html man page in a browser.

[Christian Couder]

Now when using "git help -w XXX", we will try to show the HTML man
page "git-XXX.html" in your prefered web browser.

To do that "help.c" code will call a new shell script
"git-browse-help".

This currently works only if the HTML versions of the man page
have been installed in "PREFIX/share/doc/git-doc", so new target
to do that is added to "Documentation/Makefile".

The browser to use can be configured using the "web.browser"
config variable.

We try to open a new tab in an existing web browser, if possible.

The code in "git-browse-help" is heavily stolen from "git-mergetool"
by Theodore Y. Ts'o. Thanks.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/Makefile |    4 +
 Makefile               |    4 +-
 git-browse-help.sh     |  154 ++++++++++++++++++++++++++++++++++++++++++++++++
 help.c                 |   15 ++++-
 4 files changed, 175 insertions(+), 2 deletions(-)
 create mode 100755 git-browse-help.sh

diff --git a/Documentation/Makefile b/Documentation/Makefile
index d886641..3e01718 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -29,6 +29,7 @@ DOC_MAN7=$(patsubst %.txt,%.7,$(MAN7_TXT))
 
 prefix?=$(HOME)
 bindir?=$(prefix)/bin
+htmldir?=$(prefix)/share/doc/git-doc
 mandir?=$(prefix)/share/man
 man1dir=$(mandir)/man1
 man5dir=$(mandir)/man5
@@ -95,6 +96,9 @@ install-info: info
 	  echo "No directory found in $(DESTDIR)$(infodir)" >&2 ; \
 	fi
 
+install-html: html
+	sh ./install-webdoc.sh $(htmldir)
+
 ../GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
 	$(MAKE) -C ../ GIT-VERSION-FILE
 
diff --git a/Makefile b/Makefile
index a5a40ce..9204bfe 100644
--- a/Makefile
+++ b/Makefile
@@ -223,7 +223,8 @@ SCRIPT_SH = \
 	git-merge-resolve.sh \
 	git-lost-found.sh git-quiltimport.sh git-submodule.sh \
 	git-filter-branch.sh \
-	git-stash.sh
+	git-stash.sh \
+	git-browse-help.sh
 
 SCRIPT_PERL = \
 	git-add--interactive.perl \
@@ -807,6 +808,7 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 	    -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
 	    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+	    -e 's|@@PREFIX@@|$(prefix_SQ)|g' \
 	    $@.sh >$@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
diff --git a/git-browse-help.sh b/git-browse-help.sh
new file mode 100755
index 0000000..11f8bfa
--- /dev/null
+++ b/git-browse-help.sh
@@ -0,0 +1,154 @@
+#!/bin/sh
+#
+# This program launch a web browser on the html page
+# describing a git command.
+#
+# Copyright (c) 2007 Christian Couder
+# Copyright (c) 2006 Theodore Y. Ts'o
+#
+# This file is heavily stolen from git-mergetool.sh, by
+# Theodore Y. Ts'o (thanks) that is:
+#
+# Copyright (c) 2006 Theodore Y. Ts'o
+#
+# This file is licensed under the GPL v2, or a later version
+# at the discretion of Junio C Hamano or any other official
+# git maintainer.
+#
+
+USAGE='[--browser=browser|--tool=browser] [cmd to display] ...'
+SUBDIRECTORY_OK=Yes
+OPTIONS_SPEC=
+. git-sh-setup
+
+PREFIX="@@PREFIX@@"
+GIT_VERSION="@@GIT_VERSION@@"
+
+# Directories that may contain html documentation:
+install_html_dir="$PREFIX/share/doc/git-doc"
+rpm_dir="$PREFIX/share/doc/git-core-$GIT_VERSION"
+
+# Look for the directory that really contains html documentation.
+html_dir=''
+for dir in "$install_html_dir" "$rpm_dir"
+do
+	test -d "$dir" && { html_dir="$dir" ; break ; }
+done
+test -n "$html_dir" || die "No documentation directory found."
+
+valid_tool() {
+	case "$1" in
+		firefox | iceweasel | konqueror | w3m | links | lynx | dillo)
+			;; # happy
+		*)
+			return 1
+			;;
+	esac
+}
+
+init_browser_path() {
+	browser_path=`git config browser.$1.path`
+	test -z "$browser_path" && browser_path=$1
+}
+
+while test $# != 0
+do
+    case "$1" in
+	-b|--browser*|-t|--tool*)
+	    case "$#,$1" in
+		*,*=*)
+		    browser=`expr "z$1" : 'z-[^=]*=\(.*\)'`
+		    ;;
+		1,*)
+		    usage ;;
+		*)
+		    browser="$2"
+		    shift ;;
+	    esac
+	    ;;
+	--)
+	    break
+	    ;;
+	-*)
+	    usage
+	    ;;
+	*)
+	    break
+	    ;;
+    esac
+    shift
+done
+
+if test -z "$browser"; then
+    browser=`git config web.browser`
+    if test -n "$browser" && ! valid_tool "$browser"; then
+	    echo >&2 "git config option web.browser set to unknown browser: $browser"
+	    echo >&2 "Resetting to default..."
+	    unset browser
+    fi
+fi
+
+if test -z "$browser" ; then
+    if test -n "$DISPLAY"; then
+        browser_candidates="firefox iceweasel konqueror w3m links lynx dillo"
+        if test "$KDE_FULL_SESSION" = "true"; then
+            browser_candidates="konqueror $browser_candidates"
+        fi
+    else
+        browser_candidates="w3m links lynx"
+    fi
+    echo "browser candidates: $browser_candidates"
+    for i in $browser_candidates; do
+        init_browser_path $i
+        if type "$browser_path" > /dev/null 2>&1; then
+            browser=$i
+            break
+        fi
+    done
+    test -z "$browser" && die "No known browser available."
+else
+    valid_tool "$browser" || die "Unknown browser '$browser'."
+
+    init_browser_path "$browser"
+
+    if ! type "$browser_path" > /dev/null 2>&1; then
+        die "The browser $browser is not available as '$browser_path'."
+    fi
+fi
+
+pages=$(for p in "$@"; do echo "$html_dir/$p.html" ; done)
+test -z "$pages" && pages="$html_dir/git.html"
+
+case "$browser" in
+    firefox|iceweasel)
+        # Check version because firefox < 2.0 does not support "-new-tab".
+	vers=$(expr "$($browser_path -version)" : '.* \([0-9][0-9]*\)\..*')
+	NEWTAB='-new-tab'
+	test "$vers" -lt 2 && NEWTAB=''
+        nohup "$browser_path" $NEWTAB $pages &
+	;;
+    konqueror)
+	case "$(basename "$browser_path")" in
+	    konqueror)
+		# It's simpler to use kfmclient to open a new tab in konqueror.
+		browser_path="$(echo "$browser_path" | sed -e 's/konqueror$/kfmclient/')"
+		type "$browser_path" > /dev/null 2>&1 || die "No '$browser_path' found."
+		eval "$browser_path" newTab $pages
+		;;
+	    kfmclient)
+		eval "$browser_path" newTab $pages
+		;;
+	    *)
+	        nohup "$browser_path" $pages &
+		;;
+	esac
+        ;;
+    w3m|links|lynx)
+        eval "$browser_path" $pages
+        ;;
+    dillo)
+        nohup "$browser_path" $pages &
+        ;;
+esac
+
+
diff --git a/help.c b/help.c
index 0f1cb71..ecc8c66 100644
--- a/help.c
+++ b/help.c
@@ -265,6 +265,12 @@ static void show_info_page(const char *git_cmd)
 	execlp("info", "info", page, NULL);
 }
 
+static void show_html_page(const char *git_cmd)
+{
+	const char *page = cmd_to_page(git_cmd);
+	execlp("git-browse-help", "git-browse-help", page, NULL);
+}
+
 void help_unknown_cmd(const char *cmd)
 {
 	fprintf(stderr, "git: '%s' is not a git-command. See 'git --help'.\n", cmd);
@@ -297,7 +303,14 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	const char *help_cmd = argc > 1 ? argv[1] : NULL;
 	check_help_cmd(help_cmd);
 
-	if (!strcmp(help_cmd, "--info") || !strcmp(help_cmd, "-i")) {
+	if (!strcmp(help_cmd, "--web") || !strcmp(help_cmd, "-w")) {
+		help_cmd = argc > 2 ? argv[2] : NULL;
+		check_help_cmd(help_cmd);
+
+		show_html_page(help_cmd);
+	}
+
+	else if (!strcmp(help_cmd, "--info") || !strcmp(help_cmd, "-i")) {
 		help_cmd = argc > 2 ? argv[2] : NULL;
 		check_help_cmd(help_cmd);
 
-- 
1.5.3.6.1993.g154f-dirty

Re: [PATCH 2/3] git-help: add -w|--web option to display html man page in a browser.

[Junio C Hamano]

Christian Couder <chriscool@tuxfamily.org> writes:

> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index d886641..3e01718 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -29,6 +29,7 @@ DOC_MAN7=$(patsubst %.txt,%.7,$(MAN7_TXT))
>  
>  prefix?=$(HOME)
>  bindir?=$(prefix)/bin
> +htmldir?=$(prefix)/share/doc/git-doc
>  mandir?=$(prefix)/share/man
>  man1dir=$(mandir)/man1
>  man5dir=$(mandir)/man5

Doing this and then ...

> diff --git a/Makefile b/Makefile
> index a5a40ce..9204bfe 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -807,6 +808,7 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
>  	    -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
>  	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
>  	    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
> +	    -e 's|@@PREFIX@@|$(prefix_SQ)|g' \
>  	    $@.sh >$@+ && \
>  	chmod +x $@+ && \
>  	mv $@+ $@
> ...
> diff --git a/git-browse-help.sh b/git-browse-help.sh
> new file mode 100755
> index 0000000..11f8bfa
> --- /dev/null
> +++ b/git-browse-help.sh
> @@ -0,0 +1,154 @@
> +#!/bin/sh
> ...
> +USAGE='[--browser=browser|--tool=browser] [cmd to display] ...'
> +SUBDIRECTORY_OK=Yes
> +OPTIONS_SPEC=
> +. git-sh-setup
> +
> +PREFIX="@@PREFIX@@"
> +GIT_VERSION="@@GIT_VERSION@@"
> +
> +# Directories that may contain html documentation:
> +install_html_dir="$PREFIX/share/doc/git-doc"
> +rpm_dir="$PREFIX/share/doc/git-core-$GIT_VERSION"

... doing this is wrong. People can set htmldir to somewhere other than
$(prefix)/share/doc/git-doc while building and installing, but you are
not telling the munged script where it is.

> +init_browser_path() {
> +	browser_path=`git config browser.$1.path`
> +	test -z "$browser_path" && browser_path=$1
> +}

Please do not contaminate the config file with something the user can
easily use a lot more standardized way (iow $PATH) to configure to his
taste.

I'd suggest dropping this bit.

Re: [PATCH 2/3] git-help: add -w|--web option to display html man page in a browser.

[Junio C Hamano]

Christian Couder <chriscool@tuxfamily.org> writes:

> diff --git a/help.c b/help.c
> index 0f1cb71..ecc8c66 100644
> --- a/help.c
> +++ b/help.c
> @@ -297,7 +303,14 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>  	const char *help_cmd = argc > 1 ? argv[1] : NULL;
>  	check_help_cmd(help_cmd);
>  
> -	if (!strcmp(help_cmd, "--info") || !strcmp(help_cmd, "-i")) {
> +	if (!strcmp(help_cmd, "--web") || !strcmp(help_cmd, "-w")) {
> +		help_cmd = argc > 2 ? argv[2] : NULL;
> +		check_help_cmd(help_cmd);
> +
> +		show_html_page(help_cmd);
> +	}
> +
> +	else if (!strcmp(help_cmd, "--info") || !strcmp(help_cmd, "-i")) {
>  		help_cmd = argc > 2 ? argv[2] : NULL;
>  		check_help_cmd(help_cmd);

Isn't this "check-help-cmd" duplication ugly, by the way?

Re: [PATCH 2/3] git-help: add -w|--web option to display html man page in a browser.

[Christian Couder]

Le jeudi 6 décembre 2007, Junio C Hamano a écrit :
> Christian Couder <chriscool@tuxfamily.org> writes:
> > diff --git a/Documentation/Makefile b/Documentation/Makefile
> > index d886641..3e01718 100644
> > --- a/Documentation/Makefile
> > +++ b/Documentation/Makefile
> > @@ -29,6 +29,7 @@ DOC_MAN7=$(patsubst %.txt,%.7,$(MAN7_TXT))
> >
> >  prefix?=$(HOME)
> >  bindir?=$(prefix)/bin
> > +htmldir?=$(prefix)/share/doc/git-doc
> >  mandir?=$(prefix)/share/man
> >  man1dir=$(mandir)/man1
> >  man5dir=$(mandir)/man5
>
> Doing this and then ...
>
> > diff --git a/Makefile b/Makefile
> > index a5a40ce..9204bfe 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -807,6 +808,7 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
> >  	    -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
> >  	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
> >  	    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
> > +	    -e 's|@@PREFIX@@|$(prefix_SQ)|g' \
> >  	    $@.sh >$@+ && \
> >  	chmod +x $@+ && \
> >  	mv $@+ $@
> > ...
> > diff --git a/git-browse-help.sh b/git-browse-help.sh
> > new file mode 100755
> > index 0000000..11f8bfa
> > --- /dev/null
> > +++ b/git-browse-help.sh
> > @@ -0,0 +1,154 @@
> > +#!/bin/sh
> > ...
> > +USAGE='[--browser=browser|--tool=browser] [cmd to display] ...'
> > +SUBDIRECTORY_OK=Yes
> > +OPTIONS_SPEC=
> > +. git-sh-setup
> > +
> > +PREFIX="@@PREFIX@@"
> > +GIT_VERSION="@@GIT_VERSION@@"
> > +
> > +# Directories that may contain html documentation:
> > +install_html_dir="$PREFIX/share/doc/git-doc"
> > +rpm_dir="$PREFIX/share/doc/git-core-$GIT_VERSION"
>
> ... doing this is wrong. People can set htmldir to somewhere other than
> $(prefix)/share/doc/git-doc while building and installing, but you are
> not telling the munged script where it is.

Yeah, I sent a fix for this.

> > +init_browser_path() {
> > +	browser_path=`git config browser.$1.path`
> > +	test -z "$browser_path" && browser_path=$1
> > +}
>
> Please do not contaminate the config file with something the user can
> easily use a lot more standardized way (iow $PATH) to configure to his
> taste.
>
> I'd suggest dropping this bit.

I stole this part from "git-mergetool.sh":

init_merge_tool_path() {
        merge_tool_path=`git config mergetool.$1.path`
        if test -z "$merge_tool_path" ; then
                case "$1" in
                        emerge)
                                merge_tool_path=emacs
                                ;;
                        *)
                                merge_tool_path=$1
                                ;;
                esac
        fi
}

So we should either drop it in "git-mergetool.sh" too or keep it in both 
scripts.

Thanks,
Christian.

Re: [PATCH 2/3] git-help: add -w|--web option to display html man page in a browser.

[Junio C Hamano]

Christian Couder <chriscool@tuxfamily.org> writes:

>> > +# Directories that may contain html documentation:
>> > +install_html_dir="$PREFIX/share/doc/git-doc"
>> > +rpm_dir="$PREFIX/share/doc/git-core-$GIT_VERSION"
>>
>> ... doing this is wrong. People can set htmldir to somewhere other than
>> $(prefix)/share/doc/git-doc while building and installing, but you are
>> not telling the munged script where it is.
>
> Yeah, I sent a fix for this.

Why do you even need to fallback?  I'd rather drop these two fallbacks
entirely.

Distros have their own html documentation layout policy, so I suspect
they will patch this part to their liking anyway, and this point will
mostly become moot.  For source distribution, I'd prefer to point at
the place we know we are installing in.

>> > +init_browser_path() {
>> > +	browser_path=`git config browser.$1.path`
>> > +	test -z "$browser_path" && browser_path=$1
>> > +}
>>
>> Please do not contaminate the config file with something the user can
>> easily use a lot more standardized way (iow $PATH) to configure to his
>> taste.
>>
>> I'd suggest dropping this bit.
>
> I stole this part from "git-mergetool.sh":
>
> init_merge_tool_path() {
>         merge_tool_path=`git config mergetool.$1.path`
>         if test -z "$merge_tool_path" ; then
>                 case "$1" in
>                         emerge)
>                                 merge_tool_path=emacs
>                                 ;;
> ...
> }
>
> So we should either drop it in "git-mergetool.sh" too or keep it in both 
> scripts.

I think this is an irrelevant defense.  If others are doing bad, that is
not a justification to make things worse.

In the case of mergetool, it has case "$merge_tool" that can be spelled
totally differently from path (e.g. emerge and emacs), so that function
itself is semi justified.  For browser I do not think there isn't such
justification.

Re: [PATCH 2/3] git-help: add -w|--web option to display html man page in a browser.

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Christian Couder <chriscool@tuxfamily.org> writes:
> ...
>>> > +init_browser_path() {
>>> > +	browser_path=`git config browser.$1.path`
>>> > +	test -z "$browser_path" && browser_path=$1
>>> > +}
>>>
>>> Please do not contaminate the config file with something the user can
>>> easily use a lot more standardized way (iow $PATH) to configure to his
>>> taste.
>>>
>>> I'd suggest dropping this bit.

Well, I changed my mind.  It is a bit funny to have both firefox and
iceweasel as "valid-tool", but if we consider $browser to define the
external interface and $browser_path to define the implementation, it
sort of makes sense to have that configuration.  browser_path could be
iceweasel for browser firefox.

I'll squash the patch to update the one from the last round (as the last
two patches are not yet accepted in 'next' yet), remove the html
documentation path fallback, but will leave this part in.

browser.*.path and web.browser configuration need to be documented, if
not already, though.

Re: [PATCH 2/3] git-help: add -w|--web option to display html man page in a browser.

[Christian Couder]

Le vendredi 7 décembre 2007, Junio C Hamano a écrit :
> Junio C Hamano <gitster@pobox.com> writes:
> > Christian Couder <chriscool@tuxfamily.org> writes:
> > ...
> >
> >>> > +init_browser_path() {
> >>> > +	browser_path=`git config browser.$1.path`
> >>> > +	test -z "$browser_path" && browser_path=$1
> >>> > +}
> >>>
> >>> Please do not contaminate the config file with something the user can
> >>> easily use a lot more standardized way (iow $PATH) to configure to
> >>> his taste.
> >>>
> >>> I'd suggest dropping this bit.
>
> Well, I changed my mind.  It is a bit funny to have both firefox and
> iceweasel as "valid-tool", but if we consider $browser to define the
> external interface and $browser_path to define the implementation, it
> sort of makes sense to have that configuration.  browser_path could be
> iceweasel for browser firefox.
>
> I'll squash the patch to update the one from the last round (as the last
> two patches are not yet accepted in 'next' yet), remove the html
> documentation path fallback, but will leave this part in.

Thanks.

> browser.*.path and web.browser configuration need to be documented, if
> not already, though.

Did you see this patch:

http://article.gmane.org/gmane.comp.version-control.git/67101

Christian.

Re: [PATCH 2/3] git-help: add -w|--web option to display html man page in a browser.

[Junio C Hamano]

Christian Couder <chriscool@tuxfamily.org> writes:

> diff --git a/help.c b/help.c
> index 0f1cb71..ecc8c66 100644
> --- a/help.c
> +++ b/help.c
> @@ -265,6 +265,12 @@ static void show_info_page(const char *git_cmd)
>  	execlp("info", "info", page, NULL);
>  }
>  
> +static void show_html_page(const char *git_cmd)
> +{
> +	const char *page = cmd_to_page(git_cmd);
> +	execlp("git-browse-help", "git-browse-help", page, NULL);
> +}
> +

This should be execl_git_cmd() to honor GIT_TRACE and to help transition
to bindir != gitexecdir layout, I think.