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 functional options to allow configuring the underlying Cmd #30

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

landism
Copy link

@landism landism commented Apr 16, 2021

Problem

browser exposes the Stdout and Stderr vars for consuming packages to control where its output goes. This global state serves most purposes, but doesn't suffice if a project wants different output destinations for different OpenURL calls.

Solution

Add options to the Open* functions to allow the user to configure the command's stdout and stderr.

Notes

  • It seems unfortunate that cmdOptions will be ignored on Windows, but the same was already true of the package's Stdout/Stderr vars. It seems quite possible the syscall used on windows doesn't have output anyway, but I'm not very familiar with it.
  • It's possible that changing stdout/stderr is really the only interesting thing one might want to do with CmdOption, so maybe it'd make more sense to just define an OpenOption type that just has stdout/stderr and keep exec.Command out of the API.
  • The changes to go.mod/go.sum occurred when I built the package with GOOS=windows. It looks like they should have been included in Windows: utilize ShellExecuteW #27. If you'd like, I'm happy to put them up as a separate PR since they're not really part of this change, or they're also taken care of by Fix running project on Windows #29, if that ever gets merged.

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.

1 participant