{"thread":{"id":"22097","subject":"[PATCH/RFC] Add support for the google-chrome web browser","startedAt":"2010-01-05T05:19:06Z","lastAt":"2010-01-05T19:56:40Z","messageCount":6,"participants":["Evan Farrer","Junio C Hamano","Avery Pennarun"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"130815","messageId":"20100105051906.GA22799@efarrerlx.appsig.com","threadId":"22097","inReplyTo":null,"subject":"[PATCH/RFC] Add support for the google-chrome web browser","fromName":"Evan Farrer","fromEmail":"evan.farrer@gmail.com","sentAt":"2010-01-05T05:19:06Z","receivedAt":"2010-01-05T05:19:06Z","isPatch":true,"sender":{"key":"evan.farrer@gmail.com","avatar":null},"body":"Signed-off-by: Evan Farrer \n---\n Documentation/git-web--browse.txt | 1 +\n git-web--browse.sh | 6 +++---\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-web--browse.txt b/Documentation/git-web--browse.txt\nindex 278cf73..0994139 100644\n--- a/Documentation/git-web--browse.txt\n+++ b/Documentation/git-web--browse.txt\n@@ -25,6 +25,7 @@ The following browsers (or commands) are currently supported:\n * links\n * lynx\n * dillo\n+* google-chrome\n * open (this is the default under Mac OS X GUI)\n * start (this is the default under MinGW)\n \ndiff --git a/git-web--browse.sh b/git-web--browse.sh\nindex a578c3a..0664b9e 100755\n--- a/git-web--browse.sh\n+++ b/git-web--browse.sh\n@@ -31,7 +31,7 @@ valid_custom_tool()\n \n valid_tool() {\n \tcase \"$1\" in\n-\t\tfirefox | iceweasel | konqueror | w3m | links | lynx | dillo | open | start)\n+\t\tfirefox | iceweasel | konqueror | w3m | links | lynx | dillo | google-chrome | open | start)\n \t\t\t;; # happy\n \t\t*)\n \t\t\tvalid_custom_tool \"$1\" || return 1\n@@ -103,7 +103,7 @@ fi\n \n if test -z \"$browser\" ; then\n if test -n \"$DISPLAY\"; then\n-\tbrowser_candidates=\"firefox iceweasel konqueror w3m links lynx dillo\"\n+\tbrowser_candidates=\"firefox iceweasel konqueror w3m links lynx dillo google-chrome\"\n \tif test \"$KDE_FULL_SESSION\" = \"true\"; then\n \t browser_candidates=\"konqueror $browser_candidates\"\n \tfi\n@@ -162,7 +162,7 @@ case \"$browser\" in\n \t\t;;\n \tesac\n \t;;\n- w3m|links|lynx|open)\n+ w3m|links|lynx|google-chrome|open)\n \teval \"$browser_path\" \"$@\"\n \t;;\n start)\n-- \n1.6.6.78.gbd757c\n"},{"id":"130817","messageId":"7vr5q56rew.fsf@alter.siamese.dyndns.org","threadId":"22097","inReplyTo":"20100105051906.GA22799@efarrerlx.appsig.com","subject":"Re: [PATCH/RFC] Add support for the google-chrome web browser","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-05T05:40:07Z","receivedAt":"2010-01-05T05:40:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Evan Farrer writes:\n\n> Signed-off-by: Evan Farrer \n> ---\n> Documentation/git-web--browse.txt | 1 +\n> git-web--browse.sh | 6 +++---\n> 2 files changed, 4 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/git-web--browse.txt b/Documentation/git-web--browse.txt\n> index 278cf73..0994139 100644\n> --- a/Documentation/git-web--browse.txt\n> +++ b/Documentation/git-web--browse.txt\n> @@ -25,6 +25,7 @@ The following browsers (or commands) are currently supported:\n> * links\n> * lynx\n> * dillo\n> +* google-chrome\n\nHmmm, the support added by this patch for the chrome is, together with\nw3m, links, and open, to simply run the command with given arguments\nwithout any other magic.\n\nPerhaps valid_custom_tool() function should be changed to simply return $1\nif $browser_cmd is not found, like this:\n\n\tvalid_custom_tool ()\n {\n \tbrowser_cmd=$(git config \"browser.$1.cmd\")\n : ${browser_cmdn:=$1}\n\t}\n\nThen we don't even have to add any specific support for \"google-chrome\" or\nanything that takes \"$command $path...\" and opens the documents.\n\nIs there a downside in this approach?\n"},{"id":"130848","messageId":"32541b131001050920j2a569ec8u8811f1b0358f21ef@mail.gmail.com","threadId":"22097","inReplyTo":"7vr5q56rew.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] Add support for the google-chrome web browser","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2010-01-05T17:20:40Z","receivedAt":"2010-01-05T17:20:40Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On Tue, Jan 5, 2010 at 12:40 AM, Junio C Hamano wrote:\n> Then we don't even have to add any specific support for \"google-chrome\" or\n> anything that takes \"$command $path...\" and opens the documents.\n>\n> Is there a downside in this approach?\n\nIf someone has another firefox-derived browser installed with a\ndifferent name and tries to use it, this default wouldn't do the right\n\"firefoxy\" thing, and would instead fail strangely. On the other\nhand, right now it'll fail anyway, just not strangely.\n\nSo your proposed change would decrease the number of failures, but\nincrease the strangeness of the remaining failures. Maybe that's the\nright thing to do though.\n\nAvery\n"},{"id":"130854","messageId":"7v8wccz99g.fsf@alter.siamese.dyndns.org","threadId":"22097","inReplyTo":"32541b131001050920j2a569ec8u8811f1b0358f21ef@mail.gmail.com","subject":"Re: [PATCH/RFC] Add support for the google-chrome web browser","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-05T18:39:23Z","receivedAt":"2010-01-05T18:39:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Avery Pennarun writes:\n\n> On Tue, Jan 5, 2010 at 12:40 AM, Junio C Hamano wrote:\n>> Then we don't even have to add any specific support for \"google-chrome\" or\n>> anything that takes \"$command $path...\" and opens the documents.\n>>\n>> Is there a downside in this approach?\n>\n> If someone has another firefox-derived browser installed with a\n> different name and tries to use it, this default wouldn't do the right\n> \"firefoxy\" thing, and would instead fail strangely. On the other\n> hand, right now it'll fail anyway, just not strangely.\n\nYou probably didn't notice/understand why I singled out w3m/links/open and\nexcluded firefox from the set. There is no question that the ones that\nneed custom command line need custom support. But to support a new\nbrowser that takes a bog standard \"command then args\" command line, there\nis no reason to add cruft, every time somebody comes up with a new browser.\n"},{"id":"130856","messageId":"32541b131001051056o5853b1d2xd57d75d9a62f9f1a@mail.gmail.com","threadId":"22097","inReplyTo":"7v8wccz99g.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] Add support for the google-chrome web browser","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2010-01-05T18:56:13Z","receivedAt":"2010-01-05T18:56:13Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On Tue, Jan 5, 2010 at 1:39 PM, Junio C Hamano wrote:\n> Avery Pennarun writes:\n>> On Tue, Jan 5, 2010 at 12:40 AM, Junio C Hamano wrote:\n>>> Then we don't even have to add any specific support for \"google-chrome\" or\n>>> anything that takes \"$command $path...\" and opens the documents.\n>>>\n>>> Is there a downside in this approach?\n>>\n>> If someone has another firefox-derived browser installed with a\n>> different name and tries to use it, this default wouldn't do the right\n>> \"firefoxy\" thing, and would instead fail strangely.  On the other\n>> hand, right now it'll fail anyway, just not strangely.\n>\n> You probably didn't notice/understand why I singled out w3m/links/open and\n> excluded firefox from the set.  There is no question that the ones that\n> need custom command line need custom support.  But to support a new\n> browser that takes a bog standard \"command then args\" command line, there\n> is no reason to add cruft, every time somebody comes up with a new browser.\n\nYes, I'm probably missing something, that would be normal :)\n\nMy point is that, given a random browser name, you don't know whether\nit's an easy one *or* if it needs to work more like firefox.\n\nThe current behaviour will barf right away (I think) because it\ndoesn't know. If it instead had a default case that just assumed\nnon-firefox behaviour, then it would fail *strangely* (instead of\npredictably) on browsers that needed special workarounds, such as an\nas-yet-unknown firefox derivative.\n\nMaybe this isn't important though.\n\nAvery\n"},{"id":"130857","messageId":"7veim4xr47.fsf@alter.siamese.dyndns.org","threadId":"22097","inReplyTo":"32541b131001051056o5853b1d2xd57d75d9a62f9f1a@mail.gmail.com","subject":"Re: [PATCH/RFC] Add support for the google-chrome web browser","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-05T19:56:40Z","receivedAt":"2010-01-05T19:56:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Avery Pennarun writes:\n\n> The current behaviour will barf right away (I think) because it\n> doesn't know. If it instead had a default case that just assumed\n> non-firefox behaviour, then it would fail *strangely* (instead of\n> predictably) on browsers that needed special workarounds, such as an\n> as-yet-unknown firefox derivative.\n\nOk, I can buy that.\n\nI have a feeling that extensibility situation of this script is similar to\nthat of mergetool/difftool. Perhaps a similar approach to refactor and\nthen support browser-specific peculiarities by separate files, outlined in\nhttp://thread.gmane.org/gmane.comp.version-control.git/134906/focus=135006\nmight work well, don't you think?\n"}]}