[PATCH] Improve git-help--browse browser support under OS X

[PATCH] Improve git-help--browse browser support under OS X

[jaysoffian+git]

/usr/bin/open <document> is used under OS X to open a document just as if the
user had double-clicked on the file's icon. In other words, this will open HTML
files with the user's default browser. Note however that whether the document
opens in a new window, or in a new tab depends upon the browser configuration.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 git-web--browse.sh |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/git-web--browse.sh b/git-web--browse.sh
index 8ed489d..938293a 100755
--- a/git-web--browse.sh
+++ b/git-web--browse.sh
@@ -25,7 +25,7 @@ NONGIT_OK=Yes
 
 valid_tool() {
 	case "$1" in
-		firefox | iceweasel | konqueror | w3m | links | lynx | dillo)
+		firefox | iceweasel | konqueror | w3m | links | lynx | dillo | open)
 			;; # happy
 		*)
 			return 1
@@ -104,6 +104,9 @@ if test -z "$browser" ; then
     else
 	browser_candidates="w3m links lynx"
     fi
+    if test -n "$SECURITYSESSIONID"; then
+	browser_candidates="open $browser_candidates"
+    fi
 
     for i in $browser_candidates; do
 	init_browser_path $i
@@ -147,7 +150,7 @@ case "$browser" in
 		;;
 	esac
 	;;
-    w3m|links|lynx)
+    w3m|links|lynx|open)
 	eval "$browser_path" "$@"
 	;;
     dillo)
-- 
1.5.4.1221.g7a7a1

Re: [PATCH] Improve git-help--browse browser support under OS X

[Dmitry Potapov]

On Fri, Feb 08, 2008 at 04:23:14PM -0500, jaysoffian+git@gmail.com wrote:
> /usr/bin/open <document> is used under OS X to open a document just as if the
> user had double-clicked on the file's icon. In other words, this will open HTML
> files with the user's default browser. Note however that whether the document
> opens in a new window, or in a new tab depends upon the browser configuration.

This is OS X specific, so the change should be made in such a way that it
will not affect anyone else. Because people on other platforms may have
"open" to do different things. For example, on Linux, the "open" command
is used to open a new virtual terminal. At the same time, other systems
may have their own ways to start the default browser. For instance, on
Debian based systems, it is usually x-www-browser for X and www-browser
for terminal.

Dmitry

Re: [PATCH] Improve git-help--browse browser support under OS X

[Jay Soffian]

On Feb 9, 2008 3:20 PM, Dmitry Potapov <dpotapov@gmail.com> wrote:
> On Fri, Feb 08, 2008 at 04:23:14PM -0500, jaysoffian+git@gmail.com wrote:
> > /usr/bin/open <document> is used under OS X to open a document just as if the
> > user had double-clicked on the file's icon. In other words, this will open HTML
> > files with the user's default browser. Note however that whether the document
> > opens in a new window, or in a new tab depends upon the browser configuration.
>
> This is OS X specific, so the change should be made in such a way that it
> will not affect anyone else. Because people on other platforms may have
> "open" to do different things. For example, on Linux, the "open" command
> is used to open a new virtual terminal. At the same time, other systems
> may have their own ways to start the default browser. For instance, on
> Debian based systems, it is usually x-www-browser for X and www-browser
> for terminal.

I guess I'm confused by the criticism as I thought that's what I did.
"open" is only added to the list of browsers to try if the
SECURITYSESSIONID environment variable is set (indicating an OS X GUI
login environment). I don't see how the change I made could adversely
impact the users of other systems.

j.

Re: [PATCH] Improve git-help--browse browser support under OS X

[Dmitry Potapov]

On Sat, Feb 09, 2008 at 09:15:30PM -0500, Jay Soffian wrote:
> 
> I guess I'm confused by the criticism as I thought that's what I did.
> "open" is only added to the list of browsers to try if the
> SECURITYSESSIONID environment variable is set (indicating an OS X GUI
> login environment). I don't see how the change I made could adversely
> impact the users of other systems.

Would not be better to use uname instead like this

  if test "$(uname -s)" == "Darwin"; then
    ...
or in addition to SECURITYSESSIONID:

  if test -n "$SECURITYSESSIONID" -a "$(uname -s)" == "Darwin"; then
    ...
?

I think it would be more reliable and more importantly it makes the
code easier to understand, because it is clear now for everyone that
this is OS X specific.

BTW, should not it be mentioned in the documentation? Probably, in
the list of supported web browsers in git-help.txt.

Dmitry

Re: [PATCH] Improve git-help--browse browser support under OS X

[Christian Couder]

Le dimanche 10 février 2008, Dmitry Potapov a écrit :
> On Sat, Feb 09, 2008 at 09:15:30PM -0500, Jay Soffian wrote:
> > I guess I'm confused by the criticism as I thought that's what I did.
> > "open" is only added to the list of browsers to try if the
> > SECURITYSESSIONID environment variable is set (indicating an OS X GUI
> > login environment). 

I wonder if "open" works on OS X outside the OS X GUI environment. (And do 
people use OS X outside the OS X GUI environment ?)

> > I don't see how the change I made could adversely 
> > impact the users of other systems.
>
> Would not be better to use uname instead like this
>
>   if test "$(uname -s)" == "Darwin"; then
>     ...
> or in addition to SECURITYSESSIONID:
>
>   if test -n "$SECURITYSESSIONID" -a "$(uname -s)" == "Darwin"; then
>     ...
> ?
>
> I think it would be more reliable and more importantly it makes the
> code easier to understand, because it is clear now for everyone that
> this is OS X specific.
>
> BTW, should not it be mentioned in the documentation? Probably, in
> the list of supported web browsers in git-help.txt.

In the git "next" branch, "git-help--browse" has been 
renamed "git-web--browse". And the original patch is 
against "git-web--browse" except in the title where it is 
about "git-help--browse".

Anyway now in "next", "git-web--browse" is used by both "git help" and "git 
instaweb", so the documentation of both commands need an update.

Thanks,
Christian. 

Re: [PATCH] Improve git-help--browse browser support under OS X

[Jay Soffian]

On Feb 10, 2008 7:43 AM, Dmitry Potapov <dpotapov@gmail.com> wrote:
> On Sat, Feb 09, 2008 at 09:15:30PM -0500, Jay Soffian wrote:
>
> Would not be better to use uname instead like this
>
>   if test "$(uname -s)" == "Darwin"; then
>     ...
> or in addition to SECURITYSESSIONID:
>
>   if test -n "$SECURITYSESSIONID" -a "$(uname -s)" == "Darwin"; then
>     ...
> ?
>
> I think it would be more reliable and more importantly it makes the
> code easier to understand, because it is clear now for everyone that
> this is OS X specific.

Sure.

> BTW, should not it be mentioned in the documentation? Probably, in
> the list of supported web browsers in git-help.txt.

Yep, will do.

j.

Re: [PATCH] Improve git-help--browse browser support under OS X

[Jay Soffian]

On Feb 10, 2008 8:45 AM, Christian Couder <chriscool@tuxfamily.org> wrote:

> I wonder if "open" works on OS X outside the OS X GUI environment. (And do
> people use OS X outside the OS X GUI environment ?)

"open" simulates double-clicking on a document icon, so, no it does
not make sense to use it unless the GUI is active. What I was
referring to though is that you can't ssh into another OS X host and
use open, even if there is a user logged into the GUI on that host,
for obvious security reasons.

> In the git "next" branch, "git-help--browse" has been
> renamed "git-web--browse". And the original patch is
> against "git-web--browse" except in the title where it is
> about "git-help--browse".

Yeah, my typo in the title.

> Anyway now in "next", "git-web--browse" is used by both "git help" and "git
> instaweb", so the documentation of both commands need an update.

The git instaweb documentation update should be in a separate patch.
Though I think maybe the right way to handle this is add a new doc for
git-web--browse and then have the instaweb and help docs refer to
that, no?

BTW, what is the reason for the two-dashes in git-web--browse's name?

j.

[PATCH] git-web--browse: improve browser support under OS X

[jaysoffian+git]

From: Jay Soffian <jaysoffian@gmail.com>

/usr/bin/open <document> is used under OS X to open a document as if the
user had double-clicked on the file's icon (i.e. HTML files are opened
w/the user's default browser).

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 Documentation/git-help.txt |    1 +
 git-web--browse.sh         |    7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index ad41aab..897868a 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -68,6 +68,7 @@ The following browsers are currently supported by 'git-web--browse':
 * links
 * lynx
 * dillo
+* open (OS X only, where it is the default)
 
 CONFIGURATION VARIABLES
 -----------------------
diff --git a/git-web--browse.sh b/git-web--browse.sh
index 8ed489d..1927c3b 100755
--- a/git-web--browse.sh
+++ b/git-web--browse.sh
@@ -25,7 +25,7 @@ NONGIT_OK=Yes
 
 valid_tool() {
 	case "$1" in
-		firefox | iceweasel | konqueror | w3m | links | lynx | dillo)
+		firefox | iceweasel | konqueror | w3m | links | lynx | dillo | open)
 			;; # happy
 		*)
 			return 1
@@ -104,6 +104,9 @@ if test -z "$browser" ; then
     else
 	browser_candidates="w3m links lynx"
     fi
+    if test "$(uname)" = "Darwin" && test -n "$SECURITYSESSIONID"; then
+	browser_candidates="open $browser_candidates"
+    fi
 
     for i in $browser_candidates; do
 	init_browser_path $i
@@ -147,7 +150,7 @@ case "$browser" in
 		;;
 	esac
 	;;
-    w3m|links|lynx)
+    w3m|links|lynx|open)
 	eval "$browser_path" "$@"
 	;;
     dillo)
-- 
1.5.4.1221.gf32f1

Re: [PATCH] git-web--browse: improve browser support under OS X

[Junio C Hamano]

Because we preprocess git-web--browse.sh to create the final
git-web--browse to be installed, something like "open" that is
not a user but a platform preference might be better done as a
build-time thing, instead of forcing everybody to spawn "uname",
perhaps like this?

Also I wonder why the check for SECURITYSESSIONID your original
had is not good enough for this Mac specific customization?


 Makefile           |    2 ++
 git-web--browse.sh |   18 +++++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index d288378..5c278d2 100644
--- a/Makefile
+++ b/Makefile
@@ -419,6 +419,7 @@ ifeq ($(uname_S),Darwin)
 	endif
 	NO_STRLCPY = YesPlease
 	NO_MEMMEM = YesPlease
+	PLATFORM_BROWSER = open
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease
@@ -840,6 +841,7 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 	    -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
 	    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+	    -e 's/@@PLATFORM_BROWSER@@/$(PLATFORM_BROWSER)/g' \
 	    $@.sh >$@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
diff --git a/git-web--browse.sh b/git-web--browse.sh
index 3ade500..61a3514 100755
--- a/git-web--browse.sh
+++ b/git-web--browse.sh
@@ -27,6 +27,8 @@ valid_tool() {
 	case "$1" in
 		firefox | iceweasel | konqueror | w3m | links | lynx | dillo)
 			;; # happy
+		"@@PLATFORM_BROWSER@@" )
+			;;
 		*)
 			return 1
 			;;
@@ -105,6 +107,20 @@ if test -z "$browser" ; then
 	browser_candidates="w3m links lynx"
     fi
 
+    # Test if the current environment is suitable for the
+    # platform preference and add it as the default
+
+    if	case "@@PLATFORM_BROWSER@@" in
+	open)
+		test -n "$SECURITYSESSIONID" ;;
+	# other platform preference come here...
+	*)
+		false ;;
+	esac
+    then
+	browser_candidates="@@PLATFORM_BROWSER@@ $browser_candidates"
+    fi
+
     for i in $browser_candidates; do
 	init_browser_path $i
 	if type "$browser_path" > /dev/null 2>&1; then
@@ -147,7 +163,7 @@ case "$browser" in
 		;;
 	esac
 	;;
-    w3m|links|lynx)
+    w3m|links|lynx|open)
 	eval "$browser_path" "$@"
 	;;
     dillo)

Re: [PATCH] Improve git-help--browse browser support under OS X

[Christian Couder]

Le lundi 11 février 2008, Jay Soffian a écrit :
> On Feb 10, 2008 8:45 AM, Christian Couder <chriscool@tuxfamily.org> wrote:
> > I wonder if "open" works on OS X outside the OS X GUI environment. (And
> > do people use OS X outside the OS X GUI environment ?)
>
> "open" simulates double-clicking on a document icon, so, no it does
> not make sense to use it unless the GUI is active. What I was
> referring to though is that you can't ssh into another OS X host and
> use open, even if there is a user logged into the GUI on that host,
> for obvious security reasons.

Ok, thanks for this information.

> > In the git "next" branch, "git-help--browse" has been
> > renamed "git-web--browse". And the original patch is
> > against "git-web--browse" except in the title where it is
> > about "git-help--browse".
>
> Yeah, my typo in the title.
>
> > Anyway now in "next", "git-web--browse" is used by both "git help" and
> > "git instaweb", so the documentation of both commands need an update.
>
> The git instaweb documentation update should be in a separate patch.
> Though I think maybe the right way to handle this is add a new doc for
> git-web--browse and then have the instaweb and help docs refer to
> that, no?
>
> BTW, what is the reason for the two-dashes in git-web--browse's name?

The two-dashes means that it's an helper script, and this means that it may 
not have it's own documentation. Anyway maybe I will create a documentation 
for it and move common information from Documentation/git-help.txt and 
Documentation/git-instaweb.txt into it.

Thanks,
Christian.

Re: [PATCH] git-web--browse: improve browser support under OS X

[Jay Soffian]

On Feb 10, 2008 10:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Because we preprocess git-web--browse.sh to create the final
> git-web--browse to be installed, something like "open" that is
> not a user but a platform preference might be better done as a
> build-time thing, instead of forcing everybody to spawn "uname",
> perhaps like this?

Seems like overkill (over the original patch), but honestly, I'm happy
with whatever solution deemed appropriate, just as long as I can use
my native browser on OS X. :-)

> Also I wonder why the check for SECURITYSESSIONID your original
> had is not good enough for this Mac specific customization?

Well, I thought it was. I was just responding to feedback. I don't
have enough experience with the git project to have much of an opinion
either way.

Anyway, thanks for the pointers.

j.

Re: [PATCH] git-web--browse: improve browser support under OS X

[Jay Soffian]

On Feb 10, 2008 10:01 PM, Junio C Hamano <gitster@pobox.com> wrote:

> Also I wonder why the check for SECURITYSESSIONID your original
> had is not good enough for this Mac specific customization?

Christian,

Any thoughts on Junio's patch vs the original?

http://thread.gmane.org/gmane.comp.version-control.git/73149/focus=73354

j.

Re: [PATCH] git-web--browse: improve browser support under OS X

[Christian Couder]

Le mardi 12 février 2008, Jay Soffian a écrit :
> On Feb 10, 2008 10:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Also I wonder why the check for SECURITYSESSIONID your original
> > had is not good enough for this Mac specific customization?
>
> Christian,
>
> Any thoughts on Junio's patch vs the original?

I think the original was good and Junio's patch is a good improvement on top 
of yours.

Thanks,
Christian.

Re: [PATCH] git-web--browse: improve browser support under OS X

[Junio C Hamano]

Christian Couder <chriscool@tuxfamily.org> writes:

> Le mardi 12 février 2008, Jay Soffian a écrit :
>> On Feb 10, 2008 10:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> > Also I wonder why the check for SECURITYSESSIONID your original
>> > had is not good enough for this Mac specific customization?
>>
>> Christian,
>>
>> Any thoughts on Junio's patch vs the original?
>
> I think the original was good and Junio's patch is a good improvement on top 
> of yours.

I think the original that only checked SECURITYSESSIONID was
short, sweet and inexpensive.

Unless somebody comes up with a good reason that the environment
check for SECURITYSESSIONID would misidentify (my understanding
is that it is a MacOS specific hack^Wenvironment variable and
nobody else would have it, but I haven't found authoritative
source), I think the original from Jay is good and I can apply
it to 'master'.

I very much had trouble with the $(uname -s) thing which
unconditionally penalized everybody else, and that was the
primary reason I suggested an alternative to make it cheaper.

Re: [PATCH] git-web--browse: improve browser support under OS X

[Pieter de Bie]

On Feb 13, 2008, at 11:14 PM, Junio C Hamano wrote:
>
> Unless somebody comes up with a good reason that the environment
> check for SECURITYSESSIONID would misidentify (my understanding
> is that it is a MacOS specific hack^Wenvironment variable and
> nobody else would have it, but I haven't found authoritative
> source), I think the original from Jay is good and I can apply
> it to 'master'.

When I SSH to my mac, the SECURITYSESSIONID variable isn't set, but  
open will succeed in displaying the page.

Not sure if that is a valid concern though; how often would you do a  
remote login to your mac to display a webpage?

- Pieter

Re: [PATCH] git-web--browse: improve browser support under OS X

[Jay Soffian]

On Feb 13, 2008 5:46 PM, Pieter de Bie <pdebie@ai.rug.nl> wrote:
> When I SSH to my mac, the SECURITYSESSIONID variable isn't set, but
> open will succeed in displaying the page.
>
> Not sure if that is a valid concern though; how often would you do a
> remote login to your mac to display a webpage?

That is quite odd, but I've confirmed it under 10.5.2. In 10.5 I can
ssh to another Mac and /usr/bin/open a document. If I have a GUI login
already established on that Mac, I can see (via screen sharing) the
application is launched and the document handled. But here's the odd
part -- if I *don't* have a GUI login on that Mac, the application for
the document is *still* launched, but I guess it's running in some
sort of headless mode. I don't recall 10.4 operating this way.

But this is neither here nor there. AFAIK, SECURITYSESSIONID is unique
to OS X and it reliably indicates that the user is sitting in front of
the OS X GUI.

j.

Re: [PATCH] git-web--browse: improve browser support under OS X

[Christian Couder]

Le jeudi 14 février 2008, Jay Soffian a écrit :
> AFAIK, SECURITYSESSIONID is unique
> to OS X and it reliably indicates that the user is sitting in front of
> the OS X GUI.

Ok, then please add a comment saying that to your original patch before: 

+    if test -n "$SECURITYSESSIONID"; then
+       browser_candidates="open $browser_candidates"
+    fi

and then resend it with "Acked-by: Christian Couder 
<chriscool@tuxfamily.org>".

Thanks,
Christian.