threads / patch / 11086

patch, 3 partsgit-help: add -w|--web option to display html man page in a browser.

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

## tl;dr

8 messages between Dec 2, 2007 and Dec 8, 2007. Diffs are folded; open one to read it.

replies: 7people: 2as markdown or json

Christian Couder· Dec 2, 2007, 05:07 UTC · lore

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
Show changes to 4 files +175 −2

Documentation/Makefile, Makefile, git-browse-help.sh, help.c

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
Junio C Hamano· Dec 6, 2007, 17:03 UTC · re: Christian Couder · lore

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

Christian Couder <chriscool@tuxfamily.org> writes:
Show 12 quoted lines
> 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 ...
Show 32 quoted lines
> 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.
Christian Couder· Dec 7, 2007, 05:35 UTC · re: Junio C Hamano · lore

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

Le jeudi 6 décembre 2007, Junio C Hamano a écrit :
Show 52 quoted lines
> 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.
Show 10 quoted lines
> > +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.

Junio C Hamano· Dec 7, 2007, 06:04 UTC · re: Christian Couder · lore

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

Christian Couder <chriscool@tuxfamily.org> writes:
Show 9 quoted lines
>> > +# 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.

Show 25 quoted lines
>> > +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.

Junio C Hamano· Dec 7, 2007, 07:05 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 12 quoted lines
> 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.

Christian Couder· Dec 8, 2007, 04:00 UTC · re: Junio C Hamano · lore

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

Le vendredi 7 décembre 2007, Junio C Hamano a écrit :
Show 24 quoted lines
> 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.
Junio C Hamano· Dec 6, 2007, 17:05 UTC · re: Christian Couder · lore

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

Christian Couder <chriscool@tuxfamily.org> writes:
Show 19 quoted lines
> 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?
Junio C Hamano· Dec 8, 2007, 08:28 UTC · re: Christian Couder · lore

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

Christian Couder <chriscool@tuxfamily.org> writes:
Show 14 quoted lines
> 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.

← back to recent threads