Re: [PATCHv3 5/6] web--browse: use *www-browser if available

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 8, 2010 at 12:36 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Giuseppe Bilotta <giuseppe.bilotta@xxxxxxxxx> writes:
>
>> +     # if the linked executable doesn't match a browser name we know about,
>> +     # look at the version string
>> +
>> +     # even though most browsers (and applications, in fact) will show their
>> +     # name and version on the first line of the --version output, this is
>> +     # not true in particular for the KDE apps (e.g. konqueror and kfmclient),
>> +     # which display their name and version on the LAST line. So we cannot
>> +     # clip the version string at the first line when retrieving it. Rather,
>> +     # we keep it whole and then limit it when we know what we're dealing with.
>
> I'd be more worried about the ones that do not understand --version and
> spawn a new window.

That's indeed a potential issue (although I _do_ wonder if there are
browsers that act like that; the only one I couldn't try, on Linux,
was dillo).

>> +     verstring="$("$testexe" --version 2> /dev/null)"
>> +     browser="$(echo "$verstring" | head -n 1 | cut -f1 -d' ' | tr A-Z a-z)"
>> +     case "$browser" in
>> +             mozilla)
>
> What was the first patch in this series about again ;-)?

Oh my! LOL. Ok, I can fix that 8-D

>> +                     verstring="$(echo "$verstring" | head -n 1)"
>> +                     browser="$(echo "$verstring" | cut -f2 -d' ' | tr A-Z a-z)"
>> +                     ;;
>> +             google)
>> +                     verstring="$(echo "$verstring" | head -n 1)"
>> +                     browser="google-chrome"
>> +                     ;;
>> +             qt:)
>> +                     # konqueror, kfmclient or other KDE app
>> +                     verstring="$(echo "$verstring" | tail -n 1)"
>> +                     browser="$(echo "$verstring" | cut -f1 -d:)"
>> +                     ;;
>> +             *)
>> +                     verstring="$(echo "$verstring" | head -n 1)"
>> +                     ;;
>> +
>> +     esac
>> +     if valid_tool "$browser" ; then
>> +             browser_path="$i"
>> +             return 0
>> +     fi
>> +
>> +     echo >&2 "$testexe (detected as $browser) is not a supported browser, skipping"
>> +     browser=""
>> +     return 1
>> +}
>
> Sorry, but I simply do not think it is worth this ugliness to get slight
> customization between -new-tab, newTab, and nothingness.

I personally don't have a particular feeling for forcing the opening
in a new tab (and I might have over-engineered it, although it's not
really much more complex than the previous versions of this patchset,
it's just refactored.

If this requirement is relaxed, the patch becomes much simpler, and we
can include direct support for xdg-open, sensible-browser, htlmview &
so forth.

-- 
Giuseppe "Oblomov" Bilotta
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]