Re: [PATCHv2 6/7] web--browse: use (x-)www-browser if available

From: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Date: Dec 3, 2010, 22:45 UTC
Message-ID: <AANLkTinWD53M2VjiWgeA0Qwx3OHzR2A09Y+AB2B9o1df@mail.gmail.com>
In-Reply-To: <7vaakmmrkj.fsf@alter.siamese.dyndns.org>

On Fri, Dec 3, 2010 at 11:15 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 14 quoted lines

> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
>
>> Debian and derivatives have an alternatives-based default browser
>> configuration that uses the /usr/bin/gnome-www-browser,
>> /usr/bin/x-www-browser and /usr/bin/www-browser symlinks.
>>
>> When no browser is selected by the user and the Debian alternatives are
>> available, try to see if they are one of our recognized selection and
>> in the affermative case use it. Otherwise, warn the user about them
>> being unsupported and move on with the previous detection logic.
>
> A "please step back a bit" question.  Does the packaging guideline of
> Debian say that non-browser applications should take these links as "end
> user preference" when opening HTML pages?

I'm not sure this is an actual guideline, and I cannot find much specific information about it online. It _is_ the recommended way to set the default browser in Debian (plus the BROWSER override), but there are quite a few applications that don't actually follow it. (And FWIW, as a Debian user I find this annoying, e.g. when clicking on a link and having konqueror or iceweasel pop up instead of my preferred x-www-browser, that happens to be Opera.)

I do believe that Debian encourages the use of sensible-browser (that does the BROWSER and *www-browser check itself) rather than manually going to look at those specifications. But that means giving up the "do anything we can to open stuff in a new tab" that is among the specified purposes of git-web--browse.

> The behaviour of unconfigured git across platforms would become less
> consistent if we do this, while the behaviour of random programs on one
> particular platform (Debian) would become more consistent.

That's actually the reason why I wasn't so sure it would be appropriate for inclusion.

> I am not saying that is necessarily a bad thing.  I just want to
> understand the motivation.

The motivation is that, lacking an explicit override, I believe git web--browse should try and get the information about the preferred browser from wherever it can. In a way, the *www-browser testing is akin to the use of start in Windows or open under Mac OS X.

Of course, as I only use Debian, I am not aware of what other distributions do (if they do anything at all) to allow a user to specify a preferred browser.

An alternative approach would be to get rid of the *www-browser and BROWSER patches, and just use xdg-open if it's available. Which again raises the issue of how to enforce opening the page in a new tab.

>> +# check if a given executable is a browser we like
>> +valid_exe() {
>
> Call it valid_browser_executable or something, please.

Of course.

Show 5 quoted lines

>> +     testexe="$1"
>> +     basename=$(basename $(readlink -f "$testexe"))
>
> Are we saying "readlink" must exist on the system?  This dependency is
> new, I think.

The only other use I found of readlink is in a specific area of t/test-lib.sh, so this seems like a new requirement indeed. I can either wrap that part in a type readlink check, or I can get rid of that specific section of the test, and just leave the --version check, bringing the code back to the previous patch version.

-- 
Giuseppe "Oblomov" Bilotta