[PATCH/RFC] Add support for the google-chrome web browser

[PATCH/RFC] Add support for the google-chrome web browser

[Evan Farrer]

Signed-off-by: Evan Farrer <Evan.Farrer@gmail.com>
---
 Documentation/git-web--browse.txt |    1 +
 git-web--browse.sh                |    6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-web--browse.txt b/Documentation/git-web--browse.txt
index 278cf73..0994139 100644
--- a/Documentation/git-web--browse.txt
+++ b/Documentation/git-web--browse.txt
@@ -25,6 +25,7 @@ The following browsers (or commands) are currently supported:
 * links
 * lynx
 * dillo
+* google-chrome
 * open (this is the default under Mac OS X GUI)
 * start (this is the default under MinGW)
 
diff --git a/git-web--browse.sh b/git-web--browse.sh
index a578c3a..0664b9e 100755
--- a/git-web--browse.sh
+++ b/git-web--browse.sh
@@ -31,7 +31,7 @@ valid_custom_tool()
 
 valid_tool() {
 	case "$1" in
-		firefox | iceweasel | konqueror | w3m | links | lynx | dillo | open | start)
+		firefox | iceweasel | konqueror | w3m | links | lynx | dillo | google-chrome | open | start)
 			;; # happy
 		*)
 			valid_custom_tool "$1" || return 1
@@ -103,7 +103,7 @@ fi
 
 if test -z "$browser" ; then
     if test -n "$DISPLAY"; then
-	browser_candidates="firefox iceweasel konqueror w3m links lynx dillo"
+	browser_candidates="firefox iceweasel konqueror w3m links lynx dillo google-chrome"
 	if test "$KDE_FULL_SESSION" = "true"; then
 	    browser_candidates="konqueror $browser_candidates"
 	fi
@@ -162,7 +162,7 @@ case "$browser" in
 		;;
 	esac
 	;;
-    w3m|links|lynx|open)
+    w3m|links|lynx|google-chrome|open)
 	eval "$browser_path" "$@"
 	;;
     start)
-- 
1.6.6.78.gbd757c

Re: [PATCH/RFC] Add support for the google-chrome web browser

[Junio C Hamano]

Evan Farrer <evan.farrer@gmail.com> writes:

> Signed-off-by: Evan Farrer <Evan.Farrer@gmail.com>
> ---
>  Documentation/git-web--browse.txt |    1 +
>  git-web--browse.sh                |    6 +++---
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-web--browse.txt b/Documentation/git-web--browse.txt
> index 278cf73..0994139 100644
> --- a/Documentation/git-web--browse.txt
> +++ b/Documentation/git-web--browse.txt
> @@ -25,6 +25,7 @@ The following browsers (or commands) are currently supported:
>  * links
>  * lynx
>  * dillo
> +* google-chrome

Hmmm, the support added by this patch for the chrome is, together with
w3m, links, and open, to simply run the command with given arguments
without any other magic.

Perhaps valid_custom_tool() function should be changed to simply return $1
if $browser_cmd is not found, like this:

	valid_custom_tool ()
        {
        	browser_cmd=$(git config "browser.$1.cmd")
                : ${browser_cmdn:=$1}
	}

Then we don't even have to add any specific support for "google-chrome" or
anything that takes "$command $path..." and opens the documents.

Is there a downside in this approach?

Re: [PATCH/RFC] Add support for the google-chrome web browser

[Avery Pennarun]

On Tue, Jan 5, 2010 at 12:40 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Then we don't even have to add any specific support for "google-chrome" or
> anything that takes "$command $path..." and opens the documents.
>
> Is there a downside in this approach?

If someone has another firefox-derived browser installed with a
different name and tries to use it, this default wouldn't do the right
"firefoxy" thing, and would instead fail strangely.  On the other
hand, right now it'll fail anyway, just not strangely.

So your proposed change would decrease the number of failures, but
increase the strangeness of the remaining failures.  Maybe that's the
right thing to do though.

Avery

Re: [PATCH/RFC] Add support for the google-chrome web browser

[Junio C Hamano]

Avery Pennarun <apenwarr@gmail.com> writes:

> On Tue, Jan 5, 2010 at 12:40 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Then we don't even have to add any specific support for "google-chrome" or
>> anything that takes "$command $path..." and opens the documents.
>>
>> Is there a downside in this approach?
>
> If someone has another firefox-derived browser installed with a
> different name and tries to use it, this default wouldn't do the right
> "firefoxy" thing, and would instead fail strangely.  On the other
> hand, right now it'll fail anyway, just not strangely.

You probably didn't notice/understand why I singled out w3m/links/open and
excluded firefox from the set.  There is no question that the ones that
need custom command line need custom support.  But to support a new
browser that takes a bog standard "command then args" command line, there
is no reason to add cruft, every time somebody comes up with a new browser.

Re: [PATCH/RFC] Add support for the google-chrome web browser

[Avery Pennarun]

On Tue, Jan 5, 2010 at 1:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Avery Pennarun <apenwarr@gmail.com> writes:
>> On Tue, Jan 5, 2010 at 12:40 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Then we don't even have to add any specific support for "google-chrome" or
>>> anything that takes "$command $path..." and opens the documents.
>>>
>>> Is there a downside in this approach?
>>
>> If someone has another firefox-derived browser installed with a
>> different name and tries to use it, this default wouldn't do the right
>> "firefoxy" thing, and would instead fail strangely.  On the other
>> hand, right now it'll fail anyway, just not strangely.
>
> You probably didn't notice/understand why I singled out w3m/links/open and
> excluded firefox from the set.  There is no question that the ones that
> need custom command line need custom support.  But to support a new
> browser that takes a bog standard "command then args" command line, there
> is no reason to add cruft, every time somebody comes up with a new browser.

Yes, I'm probably missing something, that would be normal :)

My point is that, given a random browser name, you don't know whether
it's an easy one *or* if it needs to work more like firefox.

The current behaviour will barf right away (I think) because it
doesn't know.  If it instead had a default case that just assumed
non-firefox behaviour, then it would fail *strangely* (instead of
predictably) on browsers that needed special workarounds, such as an
as-yet-unknown firefox derivative.

Maybe this isn't important though.

Avery

Re: [PATCH/RFC] Add support for the google-chrome web browser

[Junio C Hamano]

Avery Pennarun <apenwarr@gmail.com> writes:

> The current behaviour will barf right away (I think) because it
> doesn't know.  If it instead had a default case that just assumed
> non-firefox behaviour, then it would fail *strangely* (instead of
> predictably) on browsers that needed special workarounds, such as an
> as-yet-unknown firefox derivative.

Ok, I can buy that.

I have a feeling that extensibility situation of this script is similar to
that of mergetool/difftool.  Perhaps a similar approach to refactor and
then support browser-specific peculiarities by separate files, outlined in
http://thread.gmane.org/gmane.comp.version-control.git/134906/focus=135006
might work well, don't you think?