Re: [PATCHv3 5/6] web--browse: use *www-browser if available

On Wed, Dec 8, 2010 at 12:36 AM, Junio C Hamano [off-list ref] wrote:

Giuseppe Bilotta [off-list ref] writes:

quoted

+     # if the linked executable doesn't match a browser name we know about,
+     # look at the version string
+
+     # even though most browsers (and applications, in fact) will show their
+     # name and version on the first line of the --version output, this is
+     # not true in particular for the KDE apps (e.g. konqueror and kfmclient),
+     # which display their name and version on the LAST line. So we cannot
+     # clip the version string at the first line when retrieving it. Rather,
+     # we keep it whole and then limit it when we know what we're dealing with.

I'd be more worried about the ones that do not understand --version and
spawn a new window.

That's indeed a potential issue (although I _do_ wonder if there are
browsers that act like that; the only one I couldn't try, on Linux,
was dillo).

quoted

+     verstring="$("$testexe" --version 2> /dev/null)"
+     browser="$(echo "$verstring" | head -n 1 | cut -f1 -d' ' | tr A-Z a-z)"
+     case "$browser" in
+             mozilla)

What was the first patch in this series about again ;-)?

Oh my! LOL. Ok, I can fix that 8-D

quoted

+                     verstring="$(echo "$verstring" | head -n 1)"
+                     browser="$(echo "$verstring" | cut -f2 -d' ' | tr A-Z a-z)"
+                     ;;
+             google)
+                     verstring="$(echo "$verstring" | head -n 1)"
+                     browser="google-chrome"
+                     ;;
+             qt:)
+                     # konqueror, kfmclient or other KDE app
+                     verstring="$(echo "$verstring" | tail -n 1)"
+                     browser="$(echo "$verstring" | cut -f1 -d:)"
+                     ;;
+             *)
+                     verstring="$(echo "$verstring" | head -n 1)"
+                     ;;
+
+     esac
+     if valid_tool "$browser" ; then
+             browser_path="$i"
+             return 0
+     fi
+
+     echo >&2 "$testexe (detected as $browser) is not a supported browser, skipping"
+     browser=""
+     return 1
+}

Sorry, but I simply do not think it is worth this ugliness to get slight
customization between -new-tab, newTab, and nothingness.

I personally don't have a particular feeling for forcing the opening
in a new tab (and I might have over-engineered it, although it's not
really much more complex than the previous versions of this patchset,
it's just refactored.

If this requirement is relaxed, the patch becomes much simpler, and we
can include direct support for xdg-open, sensible-browser, htlmview &
so forth.

-- 
Giuseppe "Oblomov" Bilotta