Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a default browser Open() for UNIX systems #2

Merged
merged 3 commits into from Feb 17, 2024
Merged

Conversation

scfarley
Copy link
Contributor

For UNIX-like systems other than Linux, add a browser Open() call. Keep Linux as-is to continue handling WSL environments.

For UNIX-like systems other than Linux, add a browser Open() call.  Keep
Linux as-is to continue handling WSL environments.
@kpym
Copy link
Owner

kpym commented Feb 17, 2024

@scfarley Thanks for your contributions 🙏

It looks like that 'linux' is included in 'unix' (check syslist.go).

So it might be easier to rename browser_linux.go to browser_unix.go ?

Not only does it look simpler, but also I don't know how go prioritizes unix and linux when compiling for linux : if it considers that both files should be used on linux system, this will cause an error (function already defined).

@kpym
Copy link
Owner

kpym commented Feb 17, 2024

As I suspected, your pull request is not compatible with the other three browser_*.go files because _unix is not a goos selector, i.e. this file is not restricted only to Unix systems.

To make your browser_unix.go available only for Unix systems that are neither linux nor darwin, you have to add the build tag at the top of the file:

//go:build !windows && !darwin && !linux

@scfarley
Copy link
Contributor Author

Good catch. I updated it to prevent darwin and linux.

@kpym
Copy link
Owner

kpym commented Feb 17, 2024

Sorry but you have also to prevent windows because as I said '_unix' prevents nothing.

@scfarley
Copy link
Contributor Author

From syslist.go, it looks like the unix build tag does not include windows. According to go help buildconstraint:

During a particular build, the following build tags are satisfied:
...
"unix", if GOOS is a Unix or Unix-like system.
...

However, you are correct. GOOS=windows go build throws an error. I need to learn how Go makes the decision to consider unix a build tag with //go:build unix but not when using file names.

@kpym kpym merged commit 2d78172 into kpym:main Feb 17, 2024
@kpym
Copy link
Owner

kpym commented Feb 17, 2024

@scfarley Thanks for your contribution !

@scfarley
Copy link
Contributor Author

@scfarley Thanks for your contribution !

Thank you for this wonderful tool!

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.

None yet

2 participants