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

From: Christian Couder <chriscool@tuxfamily.org>
Date: Feb 10, 2008, 13:45 UTC
Message-ID: <200802101445.16033.chriscool@tuxfamily.org>
In-Reply-To: <20080210124336.GH30368@dpotapov.dyndns.org>

Le dimanche 10 février 2008, Dmitry Potapov a écrit :

Show 5 quoted lines

> 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 ?)

Show 19 quoted lines

> > 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.