From: Giuseppe Bilotta <hidden> Date: 2016-06-15 22:50:10
This second issue of the web--browse cleanup and extension patchset
has seen some stylistic changes in the first patches as discussed on the
ML, and a new patch to add support for the BROWSER env var. Support for
xdg-open is still missing, due to the lack of an obvious way to suggest
opening pages in new tabs (same reason why we don't directly support
Debian's sensible-browser).
I'm quite convinced that the first 4 patches (up to the "support opera,
seamonkey and elinks" patch, inclusive) are ready for merging.
The fifth patch is slightly more extensive than in the previous
iteration, including chromium-browser as a supported option, but still
looking for chromium-browser first even when browser=chromium.
The last two patches are more RFCish. I tried to include the changes
suggested by Christian, unless of course I forgot something. Suggestions
on possible ways to improve the proposed approach are welcome.
Giuseppe Bilotta (7):
CodingGuidelines: mention whitespace preferences for shell scripts
web--browse: coding style
web--browse: split valid_tool list
web--browse: support opera, seamonkey and elinks
web--browse: better support for chromium
web--browse: use (x-)www-browser if available
web--browse: look at the BROWSER env var
Documentation/CodingGuidelines | 4 +
Documentation/git-web--browse.txt | 10 ++
git-web--browse.sh | 276 +++++++++++++++++++++++++------------
3 files changed, 203 insertions(+), 87 deletions(-)
--
1.7.3.2.664.g294b8.dirty
@@ -31,6 +31,10 @@ But if you must have a list of rules, here they are. For shell scripts specifically (not exhaustive):+ - We use tabs for indentation.++ - Case arms are not indented with respect to the case and esac lines.+ - We prefer $( ... ) for command substitution; unlike ``, it properly nests. It should have been the way Bourne spelled it from day one, but unfortunately isn't.
@@ -46,139 +46,139 @@ init_browser_path() {whiletest$#!=0do-case"$1"in+case"$1"in-b|--browser*|-t|--tool*)-case"$#,$1"in+case"$#,$1"in*,*=*)-browser=`expr"z$1":'z-[^=]*=\(.*\)'`-;;+browser=`expr"z$1":'z-[^=]*=\(.*\)'`+;;1,*)-usage;;+usage;;*)-browser="$2"-shift;;-esac-;;+browser="$2"+shift;;+esac+;;-c|--config*)-case"$#,$1"in+case"$#,$1"in*,*=*)-conf=`expr"z$1":'z-[^=]*=\(.*\)'`-;;+conf=`expr"z$1":'z-[^=]*=\(.*\)'`+;;1,*)-usage;;+usage;;*)-conf="$2"-shift;;-esac-;;+conf="$2"+shift;;+esac+;;--)-break-;;+break+;;-*)-usage-;;+usage+;;*)-break-;;-esac-shift+break+;;+esac+shiftdonetest$#=0&&usageiftest-z"$browser"then-foroptin"$conf""web.browser"-do-test-z"$opt"&&continue-browser="`git config $opt`"-test-z"$browser"||break-done-iftest-n"$browser"&&!valid_tool"$browser";then-echo>&2"git config option $opt set to unknown browser: $browser"-echo>&2"Resetting to default..."-unsetbrowser-fi+foroptin"$conf""web.browser"+do+test-z"$opt"&&continue+browser="`git config $opt`"+test-z"$browser"||break+done+iftest-n"$browser"&&!valid_tool"$browser";then+echo>&2"git config option $opt set to unknown browser: $browser"+echo>&2"Resetting to default..."+unsetbrowser+fifiiftest-z"$browser";then-iftest-n"$DISPLAY";then-browser_candidates="firefox iceweasel google-chrome chrome chromium konqueror w3m links lynx dillo"-iftest"$KDE_FULL_SESSION"="true";then-browser_candidates="konqueror $browser_candidates"+iftest-n"$DISPLAY";then+browser_candidates="firefox iceweasel google-chrome chrome chromium konqueror w3m links lynx dillo"+iftest"$KDE_FULL_SESSION"="true";then+browser_candidates="konqueror $browser_candidates"+fi+else+browser_candidates="w3m links lynx"fi-else-browser_candidates="w3m links lynx"-fi-# SECURITYSESSIONID indicates an OS X GUI login session-iftest-n"$SECURITYSESSIONID"\--o"$TERM_PROGRAM"="Apple_Terminal";then-browser_candidates="open $browser_candidates"-fi-# /bin/start indicates MinGW-iftest-x/bin/start;then-browser_candidates="start $browser_candidates"-fi--foriin$browser_candidates;do-init_browser_path$i-iftype"$browser_path">/dev/null2>&1;then-browser=$i-break+# SECURITYSESSIONID indicates an OS X GUI login session+iftest-n"$SECURITYSESSIONID"\+-o"$TERM_PROGRAM"="Apple_Terminal";then+browser_candidates="open $browser_candidates"fi-done-test-z"$browser"&&die"No known browser available."+# /bin/start indicates MinGW+iftest-x/bin/start;then+browser_candidates="start $browser_candidates"+fi++foriin$browser_candidates;do+init_browser_path$i+iftype"$browser_path">/dev/null2>&1;then+browser=$i+break+fi+done+test-z"$browser"&&die"No known browser available."else-valid_tool"$browser"||die"Unknown browser '$browser'."+valid_tool"$browser"||die"Unknown browser '$browser'."-init_browser_path"$browser"+init_browser_path"$browser"-iftest-z"$browser_cmd"&&!type"$browser_path">/dev/null2>&1;then-die"The browser $browser is not available as '$browser_path'."-fi+iftest-z"$browser_cmd"&&!type"$browser_path">/dev/null2>&1;then+die"The browser $browser is not available as '$browser_path'."+fificase"$browser"in-firefox|iceweasel)+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"-lt2&&NEWTAB=''"$browser_path"$NEWTAB"$@"&;;-google-chrome|chrome|chromium)+google-chrome|chrome|chromium)# Actual command for chromium is chromium-browser.# No need to specify newTab. It's default in chromiumeval"$browser_path""$@"&;;-konqueror)+konqueror)case"$(basename"$browser_path")"in-konqueror)+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/null2>&1||die"No '$browser_path' found."eval"$browser_path"newTab"$@";;-kfmclient)+kfmclient)eval"$browser_path"newTab"$@";;-*)+*)"$browser_path""$@"&;;esac;;-w3m|links|lynx|open)+w3m|links|lynx|open)eval"$browser_path""$@";;-start)-exec"$browser_path"'"web-browse"'"$@"-;;-dillo)+start)+exec"$browser_path"'"web-browse"'"$@"+;;+dillo)"$browser_path""$@"&;;-*)+*)iftest-n"$browser_cmd";then-(eval$browser_cmd"$@")+(eval$browser_cmd"$@")fi;;esac
From: Giuseppe Bilotta <hidden> Date: 2016-06-15 22:50:10
It was getting too long, and we want to add some more.
Signed-off-by: Giuseppe Bilotta <redacted>
---
git-web--browse.sh | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
From: Giuseppe Bilotta <hidden> Date: 2016-06-15 22:50:10
Debian and derivatives have an alternatives-based default browser
configuration that uses the /usr/bin/gnome-www-browser,
/usr/bin/x-www-browser and /usr/bin/www-browser symlinks.
When no browser is selected by the user and the Debian alternatives are
available, try to see if they are one of our recognized selection and
in the affermative case use it. Otherwise, warn the user about them
being unsupported and move on with the previous detection logic.
Signed-off-by: Giuseppe Bilotta <redacted>
---
Documentation/git-web--browse.txt | 4 ++
git-web--browse.sh | 59 +++++++++++++++++++++++++++++++++++--
2 files changed, 60 insertions(+), 3 deletions(-)
@@ -36,6 +36,10 @@ The following browsers (or commands) are currently supported: Custom commands may also be specified.+If no default browser is specified, and /usr/bin/x-www-browser+(under X) or /usr/bin/www-browser is present, they are used to determine+the browser to use.+ OPTIONS ------- -b <browser>::
@@ -49,6 +49,38 @@ init_browser_path() {test-z"$browser_path"&&browser_path="$1"}+# check if a given executable is a browser we like+valid_exe(){+testexe="$1"+basename=$(basename$(readlink-f"$testexe"))+ifvalid_tool"$basename";then+browser="$basename"+browser_path="$testexe"+return0+fi++# if the linked executable doesn't match a browser name,+# look at the version string+verstring="$("$testexe"--version2>/dev/null)"+browser="$(echo"$verstring"|head-n1|cut-f1-d' '|trA-Za-z)"+case"$browser"in+mozilla)+browser="$(echo"$verstring"|head-n1|cut-f2-d' '|trA-Za-z)"+;;+google)+browser="google-chrome"+;;+esac+ifvalid_tool"$browser";then+browser_path="$i"+return0+fi++echo>&2"$basename ($browser) is not a supported browser, skipping"+browser=""+return1+}+whiletest$#!=0docase"$1"in
@@ -106,6 +138,26 @@ thenfifi+# Debian and derivatives use gnome-www-browser, x-www-browser or www-browser to+# set the default browser for the system. If the user did not specify a tool and+# we detect that one of the *www-browser links to a supported one, we pick it.+# Otherwise, we warn the user about them being unsupported and proceed to look+# for a supported browser.+iftest-z"$browser";then+wwwbrowser="/usr/bin/www-browser"+iftest-n"$DISPLAY";then+wwwbrowser="/usr/bin/x-www-browser $wwwbrowser"+iftest-n"$GNOME_DESKTOP_SESSION_ID";then+wwwbrowser="/usr/bin/gnome-www-browser $wwwbrowser"+fi+fi+foriin$wwwbrowser;do+iftest-x$i&&valid_exe$i;then+break+fi+done+fi+iftest-z"$browser";theniftest-n"$DISPLAY";thenbrowser_candidates="firefox iceweasel google-chrome chrome chromium chromium-browser konqueror opera seamonkey iceape w3m elinks links lynx dillo"
@@ -133,7 +185,7 @@ if test -z "$browser" ; thenfidonetest-z"$browser"&&die"No known browser available."-else+elseiftest-z"$browser_path";thenvalid_tool"$browser"||die"Unknown browser '$browser'."init_browser_path"$browser"
@@ -141,12 +193,13 @@ elseiftest-z"$browser_cmd"&&!type"$browser_path">/dev/null2>&1;thendie"The browser $browser is not available as '$browser_path'."fi-fi+fificase"$browser"in firefox|iceweasel|seamonkey|iceape)# Check version because firefox < 2.0 does not support "-new-tab".-vers=$(expr"$($browser_path-version)":'.* \([0-9][0-9]*\)\..*')+test-z"$verstring"&&verstring="$($browser_path-version)"+vers=$(expr"$verstring":'.* \([0-9][0-9]*\)\..*')NEWTAB='-new-tab'test"$vers"-lt2&&NEWTAB=''"$browser_path"$NEWTAB"$@"&
From: Giuseppe Bilotta <hidden> Date: 2016-06-15 22:50:10
On Debian-based distributions, Chromium the browser is available under
the name chromium-browser rather than chromium, to prevent conflicts
with the Chromium B.S.U. game.
Look for chromium-browser first when setting the path for chromium, and
also add chromium-browser as a supported browser name. Document the
dual-name support, and mention the dual-name support for
(google-)chrome too.
Signed-off-by: Giuseppe Bilotta <redacted>
---
Documentation/git-web--browse.txt | 4 ++--
git-web--browse.sh | 11 +++++++----
2 files changed, 9 insertions(+), 6 deletions(-)
@@ -22,8 +22,8 @@ The following browsers (or commands) are currently supported: * iceweasel * seamonkey * iceape-* chromium-* google-chrome+* chromium (also supported as chromium-browser)+* google-chrome (also supported as chrome) * konqueror (this is the default under KDE, see 'Note about konqueror' below) * opera * w3m (this is the default outside graphical environments)
@@ -147,8 +151,7 @@ firefox|iceweasel|seamonkey|iceape)test"$vers"-lt2&&NEWTAB=''"$browser_path"$NEWTAB"$@"&;;-google-chrome|chrome|chromium)-# Actual command for chromium is chromium-browser.+google-chrome|chrome|chromium|chromium-browser)# No need to specify newTab. It's default in chromiumeval"$browser_path""$@"&;;
From: Giuseppe Bilotta <hidden> Date: 2016-06-15 22:50:10
The BROWSER environment variables is used in Debian-based systems to set
the user-preferred browser(s).
It contains a colon-separate list of commands to (try to) execute
to open a web page. Each item in the list is allowed to have a %s
placeholder to be replaced by the URL, in which case we try to run the
command as is. If no placeholder is found, we only look at the command
name to see if it matches one of our known browsers.
Signed-off-by: Giuseppe Bilotta <redacted>
---
git-web--browse.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 45 insertions(+), 0 deletions(-)
@@ -138,6 +138,51 @@ thenfifi+# Debian and derivatives provide a sensible-brower script that tries to+# invoke the user or system preferred browser most appropriate for the+# current situation (X or no X, GNOME or other/no DE).+# We do not use the sensible-browser script directly since it doesn't+# allow us to open pages in a new tab, but we can use the underlying+# infrastructure to find the browser to use if the user didn't choose+# one. This is done by looking at the BROWSER environment variable+# first, and at the *www-browser links if the first search is+# unsuccesful.++# The BROWSER environment variable is a colon-separate list of commands+# to (try and) execute to launch the browser. sensible-browser allows+# each BROWSER entry to contain a %s placeholder that will be replaced+# by the URL to be opened.+# If an entry contains a %s we run it as-is, without doing any detection, on+# the premise that it represents the exact way the user expects the browser to+# be called. If the execution fails, we do not bail out, since the+# failure might be due to the entry being for a graphical browser and+# the GUI not being available, which is the reason why multiple entries+# can be specified in BROWSER in the first place.+# An entry without a %s is only taken as indication of the preferred+# browser, so we proceed with our usual detection logic.+iftest-z"$browser"-a-n"$BROWSER";then+OLDIFS="$IFS"+IFS=:+foriin$BROWSER;do+case"$i"in+*sensible-browser*)+;;# skip+*%s*)+IFS="$OLDIFS"+cmd=$(printf"$i\n""$*")+$cmd&&exit0+;;+*)+prog=$(which"$i"2>/dev/null)+iftest-n"$prog"-a-x"$prog"&&valid_exe"$prog";then+break+fi+;;+esac+done+IFS="$OLDIFS"+fi+# Debian and derivatives use gnome-www-browser, x-www-browser or www-browser to# set the default browser for the system. If the user did not specify a tool and# we detect that one of the *www-browser links to a supported one, we pick it.
From: Giuseppe Bilotta <hidden> Date: 2016-06-15 22:50:10
The list of supported browsers is also updated in the documentation.
Signed-off-by: Giuseppe Bilotta <redacted>
---
Documentation/git-web--browse.txt | 6 ++++++
git-web--browse.sh | 14 +++++++-------
2 files changed, 13 insertions(+), 7 deletions(-)
@@ -20,8 +20,14 @@ The following browsers (or commands) are currently supported: * firefox (this is the default under X Window when not using KDE) * iceweasel+* seamonkey+* iceape+* chromium+* google-chrome * konqueror (this is the default under KDE, see 'Note about konqueror' below)+* opera * w3m (this is the default outside graphical environments)+* elinks * links * lynx * dillo
@@ -140,7 +140,7 @@ elseficase"$browser"in-firefox|iceweasel)+firefox|iceweasel|seamonkey|iceape)# Check version because firefox < 2.0 does not support "-new-tab".vers=$(expr"$($browser_path-version)":'.* \([0-9][0-9]*\)\..*')NEWTAB='-new-tab'
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:50:10
Giuseppe Bilotta wrote:
quoted hunk
--- a/git-web--browse.sh+++ b/git-web--browse.sh
@@ -138,6 +138,51 @@ then
[...]
+if test -z "$browser" -a -n "$BROWSER"; then
+ OLDIFS="$IFS"
+ IFS=:
[...]
+ IFS="$OLDIFS"
Micronit: on some shells (e.g., old ash[1]), IFS starts out unset if
it hasn't been inherited in the environment. How about something like
this?
LF="
"
usual_ifs=" $LF"
IFS=:
...
IFS=$usual_ifs
...
IFS=$usual_ifs
Based on [2] I would guess that any POSIX-style shell would also
support
IFS=:
...
unset IFS
...
unset IFS
but maybe that's playing with fire.
The rest of this series looks quite nice, so
Acked-by: Jonathan Nieder <redacted>
[1] http://bugs.debian.org/95856
[2] http://www.in-ulm.de/~mascheck/various/ifs/
From: Christian Couder <hidden> Date: 2016-06-15 22:50:10
Hi,
On Fri, Dec 3, 2010 at 5:47 PM, Giuseppe Bilotta
[off-list ref] wrote:
quoted hunk
Debian and derivatives have an alternatives-based default browser
configuration that uses the /usr/bin/gnome-www-browser,
/usr/bin/x-www-browser and /usr/bin/www-browser symlinks.
When no browser is selected by the user and the Debian alternatives are
available, try to see if they are one of our recognized selection and
in the affermative case use it. Otherwise, warn the user about them
being unsupported and move on with the previous detection logic.
Signed-off-by: Giuseppe Bilotta <redacted>
---
Documentation/git-web--browse.txt | 4 ++
git-web--browse.sh | 59 +++++++++++++++++++++++++++++++++++--
2 files changed, 60 insertions(+), 3 deletions(-)
@@ -36,6 +36,10 @@ The following browsers (or commands) are currently supported:
Custom commands may also be specified.
+If no default browser is specified, and /usr/bin/x-www-browser
+(under X) or /usr/bin/www-browser is present, they are used to determine
+the browser to use.
It looks like /usr/bin/gnome-www-browser is missing.
test -z "$browser_path" && browser_path="$1"
}
+# check if a given executable is a browser we like
+valid_exe() {
+ testexe="$1"
+ basename=$(basename $(readlink -f "$testexe"))
+ if valid_tool "$basename" ; then
+ browser="$basename"
+ browser_path="$testexe"
+ return 0
+ fi
+
+ # if the linked executable doesn't match a browser name,
+ # look at the version string
+ verstring="$("$testexe" --version 2> /dev/null)"
+ browser="$(echo "$verstring" | head -n 1 | cut -f1 -d' ' | tr A-Z a-z)"
+ case "$browser" in
+ mozilla)
+ browser="$(echo "$verstring" | head -n 1 | cut -f2 -d' ' | tr A-Z a-z)"
+ ;;
+ google)
+ browser="google-chrome"
+ ;;
+ esac
+ if valid_tool "$browser" ; then
+ browser_path="$i"
+ return 0
+ fi
+
+ echo >&2 "$basename ($browser) is not a supported browser, skipping"
Why not:
echo >&2 "$basename (from $testexe) is not a supported browser, skipping"
?
Otherwise we might get something like:
"newbrowser (newbrowser) is not a supported browser, skipping"
quoted hunk
+ browser=""
+ return 1
+}
+
while test $# != 0
do
case "$1" in
@@ -106,6 +138,26 @@ then
fi
fi
+# Debian and derivatives use gnome-www-browser, x-www-browser or www-browser to
+# set the default browser for the system. If the user did not specify a tool and
+# we detect that one of the *www-browser links to a supported one, we pick it.
+# Otherwise, we warn the user about them being unsupported and proceed to look
+# for a supported browser.
+if test -z "$browser" ; then
+ wwwbrowser="/usr/bin/www-browser"
+ if test -n "$DISPLAY"; then
+ wwwbrowser="/usr/bin/x-www-browser $wwwbrowser"
+ if test -n "$GNOME_DESKTOP_SESSION_ID"; then
+ wwwbrowser="/usr/bin/gnome-www-browser $wwwbrowser"
+ fi
+ fi
+ for i in $wwwbrowser; do
+ if test -x $i && valid_exe $i ; then
+ break
+ fi
+ done
+fi
+
if test -z "$browser" ; then
if test -n "$DISPLAY"; then
browser_candidates="firefox iceweasel google-chrome chrome chromium chromium-browser konqueror opera seamonkey iceape w3m elinks links lynx dillo"
@@ -133,7 +185,7 @@ if test -z "$browser" ; then
fi
done
test -z "$browser" && die "No known browser available."
-else
+else if test -z "$browser_path"; then
Now that you reset $browser above, I am not sure this test -z
"$browser_path" is useful...
Thanks,
Christian.
From: Giuseppe Bilotta <hidden> Date: 2016-06-15 22:50:10
On Fri, Dec 3, 2010 at 6:40 PM, Christian Couder
[off-list ref] wrote:
Hi,
quoted
Custom commands may also be specified.
+If no default browser is specified, and /usr/bin/x-www-browser
+(under X) or /usr/bin/www-browser is present, they are used to determine
+the browser to use.
It looks like /usr/bin/gnome-www-browser is missing.
Good catch. I'll add it.
quoted
+ echo >&2 "$basename ($browser) is not a supported browser, skipping"
Why not:
echo >&2 "$basename (from $testexe) is not a supported browser, skipping"
?
Otherwise we might get something like:
"newbrowser (newbrowser) is not a supported browser, skipping"
I rephrased it to:
echo >&2 "$testexe (detected as $browser) is not a supported browser, skipping"
quoted
@@ -133,7 +185,7 @@ if test -z "$browser" ; then
fi
done
test -z "$browser" && die "No known browser available."
-else
+else if test -z "$browser_path"; then
Now that you reset $browser above, I am not sure this test -z
"$browser_path" is useful...
I believe it is. If the *www-browser detection was successful,
_that's_ what we want to launch, so we want to skip the
init_browser_path in that else section, which we would hit if we
didn't check for $browser_path. We also don't want to repeat the
valid_tool check, since we know it's valid already.
--
Giuseppe "Oblomov" Bilotta