Skip to content

Comments

feat: support BROWSER env var in Linux#109

Closed
bjones1 wants to merge 2 commits intoByron:mainfrom
bjones1:browser-env-var
Closed

feat: support BROWSER env var in Linux#109
bjones1 wants to merge 2 commits intoByron:mainfrom
bjones1:browser-env-var

Conversation

@bjones1
Copy link

@bjones1 bjones1 commented May 28, 2025

Per #108, this PR adds support for the BROWSER environment variable in Linux.

I've also added a Codespaces config file, which allowed me to test this easily in Codespaces.

Feedback is welcome!

@Byron
Copy link
Owner

Byron commented May 29, 2025

Apologies, but I realize this issue came up before: #8 .
There I predicted that implementing this correctly wouldn't be easy, and that it would be a major version change if put into the belly of the that() function.

However, you could add a new function that tries BROWSER first to basically open everything, then fallback to the other programs.

If this sounds like too much, I'd happily take the codespaces configuration commit.

Copy link
Owner

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to put this PR into the right state.

@bjones1
Copy link
Author

bjones1 commented May 29, 2025

Per the docs for this crate, "Use this library to open a given path or URL in some application, obeying the current user’s desktop configuration." After reading #8, I think that there's no good way to accomplish this goal on Linux -- ignoring $BROWSER causes some cases to fail, while using $BROWSER causes other cases to fail. Per #8, I think that encouraging users to use the webbrowser crate if the purpose is to open a URL in a web browser is best, instead of adding more functions that are web browser only. Therefore, I'd suggest closing this PR and instead updating the docs to clarify this point.

I'll submit the Codespaces update as a separate PR.

What are your thoughts?

@Byron
Copy link
Owner

Byron commented May 29, 2025

Therefore, I'd suggest closing this PR and instead updating the docs to clarify this point.

Yes, that sounds good. I hope you will find the time to contribute that addition as well, and agree that referring this to the webbrowser crate is preferable.

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants