search

pkg / browser

Package browser provides helpers to open files, readers, and urls in a browser window.
BSD 2-Clause "Simplified" License
559 stars 99 forks source link

Add support for the BROWSER environment variable #14

Closed teddywing closed 3 years ago

teddywing commented 5 years ago

Allow user-defined URL-opening commands via the BROWSER environment variable. This enables using a non-default browser, or changing the browser for a single command.

For example, on Mac:

$ export BROWSER='open -a Firefox'

Windows:

> setx BROWSER "start iexplore"

In UNIX environments, the command is run through $SHELL -c, or /bin/sh if $SHELL is not set. On Windows, it uses cmd /c.

Ensure that URLs are properly quoted between UNIX and Windows environments in fmtBrowserCmd().

Tested manually on Mac OS X and Windows 10.

luna-duclos commented 3 years ago

I'm not entirely sure forking through shell and cmd /c is actually safe, as such I'd rather avoid going that route entirely.

Tbh, I'm not sure opening a non default browser is in scope of this package. Whats the usecase for this ?

luna-duclos commented 3 years ago

Tidying up, Closing this as very old, please reopen if still relevant.

jetersen commented 3 years ago

This is the default way in Linux to override your browser.

luna-duclos commented 3 years ago

This is the default way in Linux to override your browser.

Shouldn't xdg-open, x-www-browser and www-browser support it directly, in that case ? (Those are the currently used binaries on linux)

mislav commented 3 years ago

I'm a maintainer of several CLI tools that support BROWSER (hub, GitHub CLI), and even though I personally like to offer users the option of per-call customizability, I would advise against respecting BROWSER in this package.

The primary reason is because there is no single spec to how BROWSER should be parsed, so programs differ in their approach to respecting the variable, prompting users to report the behavior of one tool as a bug compared to the behavior of another tool.

There is this from 2002, but due to its vagueness, it's hardly a spec. It cites Python webbrowser as one of the notable implementers, but even webbrowser implementation slightly differs from what is outlined in that document. The implementation in this PR differs even more in it that it does not support neither path separators nor %s substitution.

If tools need to respect BROWSER in a way they see fit, they can do so manually before invoking pkg/browser functionality.

teddywing commented 3 years ago

Thanks all for the comments. Your concern about going through the shell is understandable.

There is this from 2002, but due to its vagueness, it's hardly a spec. It cites Python webbrowser as one of the notable implementers, but even webbrowser implementation slightly differs from what is outlined in that document. The implementation in this PR differs even more in it that it does not support neither path separators nor %s substitution.

I appreciate the background information on this.

It seems like the BROWSER environment variable is out of scope for this package. Glad to have an answer to that.