* [PATCH/RFC] Add support for the google-chrome web browser
@ 2010-01-05 5:19 Evan Farrer
2010-01-05 5:40 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Evan Farrer @ 2010-01-05 5:19 UTC (permalink / raw)
To: git; +Cc: Christian Couder
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH/RFC] Add support for the google-chrome web browser
2010-01-05 5:19 [PATCH/RFC] Add support for the google-chrome web browser Evan Farrer
@ 2010-01-05 5:40 ` Junio C Hamano
2010-01-05 17:20 ` Avery Pennarun
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2010-01-05 5:40 UTC (permalink / raw)
To: Evan Farrer; +Cc: git, Christian Couder
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?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH/RFC] Add support for the google-chrome web browser
2010-01-05 5:40 ` Junio C Hamano
@ 2010-01-05 17:20 ` Avery Pennarun
2010-01-05 18:39 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Avery Pennarun @ 2010-01-05 17:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Evan Farrer, git, Christian Couder
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] Add support for the google-chrome web browser
2010-01-05 17:20 ` Avery Pennarun
@ 2010-01-05 18:39 ` Junio C Hamano
2010-01-05 18:56 ` Avery Pennarun
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2010-01-05 18:39 UTC (permalink / raw)
To: Avery Pennarun; +Cc: Evan Farrer, git, Christian Couder
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] Add support for the google-chrome web browser
2010-01-05 18:39 ` Junio C Hamano
@ 2010-01-05 18:56 ` Avery Pennarun
2010-01-05 19:56 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Avery Pennarun @ 2010-01-05 18:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Evan Farrer, git, Christian Couder
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] Add support for the google-chrome web browser
2010-01-05 18:56 ` Avery Pennarun
@ 2010-01-05 19:56 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2010-01-05 19:56 UTC (permalink / raw)
To: Avery Pennarun; +Cc: Evan Farrer, git, Christian Couder
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?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-01-05 19:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-05 5:19 [PATCH/RFC] Add support for the google-chrome web browser Evan Farrer
2010-01-05 5:40 ` Junio C Hamano
2010-01-05 17:20 ` Avery Pennarun
2010-01-05 18:39 ` Junio C Hamano
2010-01-05 18:56 ` Avery Pennarun
2010-01-05 19:56 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).