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

Christian Couder [off-list ref] writes:

quoted

quoted

+# 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.

quoted

quoted

+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.