auto-open a browser pointed at the gRPCui #68
Conversation
| if terminal.IsTerminal(int(os.Stdin.Fd())) { | ||
| *openBrowser = true | ||
| } | ||
|
|
There was a problem hiding this comment.
With a bool, there's no actual way to determine if the flag was set or was defaulted right? So you'll just do this unconditionally? If so, makes me wonder why make it a flag at all...
Also worth checking of stdout is a terminal instead of stdin-- stdin makes sense when deciding to prompt for confirmations. But in this case, it's really more about what you want to do with the output (the url of the running server).
There was a problem hiding this comment.
This is being set before parsing flags. So we default it to true for terminals, false otherwise. Then we parse flags so command-line can override.
I was checking stdin because it seemed the more likely indicator that we are running interactive (even though we aren't prompting for input). I could see doing grpcui api.grpc.me:443 | tee grpcui.log and still wanting that to open in a browser.
I could also just always default to false and make people remember -open-browser to save themselves from clicking "new tab" and then copying+pasting the URL...
I think for me personally, I almost always want it to open the browser window. But I know there are users (like about everyone that runs it in a docker container) that use it as a sidecar to expose a UI for some other gRPC server. But I think the sidecar case -- whether it's in a container or a service on a VM -- would almost always be deamonized and have stdin set to /dev/null.
I'm still a little on the fence if it's a good idea for this to be the default behavior. I've limited it so it does not try to do this by default if stdin is not a terminal. But I'm wondering if there are any other better signals as to when this is appropriate. I still need to double-check that, with the current logic, it won't try to do this when running in a container.
The
go.modandgo.sumfiles have changed a bit more than might be expected because I ranmake cito re-generate it. (Last time I re-created it, I had just usedgo test ./..., so it was incomplete with regards to the various tools used in CI checks.)Resolves #64