-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
cmd/pprof: select browser for web output more intelligently #10259
Comments
Yes, I agree that the defaults are in the wrong order. The platform-specific file openers should be first and the opinionated fallbacks should come later. The fix is simple: diff --git a/src/cmd/pprof/internal/commands/commands.go b/src/cmd/pprof/internal/commands/commands.go
index 51397a3..2c5e2d0 100644
--- a/src/cmd/pprof/internal/commands/commands.go
+++ b/src/cmd/pprof/internal/commands/commands.go
@@ -82,7 +82,8 @@ func PProf(c Completer, interactive **bool, svgpan **string) Commands {
// browsers returns a list of commands to attempt for web visualization
// on the current platform
func browsers() []string {
- cmds := []string{"chrome", "google-chrome", "firefox"}
+ var cmds []string
+
switch runtime.GOOS {
case "darwin":
cmds = append(cmds, "/usr/bin/open")
@@ -91,7 +92,7 @@ func browsers() []string {
default:
cmds = append(cmds, "xdg-open")
}
- return cmds
+ return append(cmds, "chrome", "google-chrome", "firefox")
} Any other opinions? If ok, I will send a CL. |
I suspect the fallbacks will never be used in this case (well, unless the OS opener does not exist). We could potentially get rid of them altogether? |
I would keep them, since I don't know how dragonfly or plan9 implements that. |
Wouldn't a preferred fallback to |
I honestly don't know, so I suggested the minimal fix. |
My guess for best order of operations would be ["os-specific opener", The fallback list also has other problems like the "google-chrome" binary sometimes being called "google-chrome-stable", beta versions of browsers are never used, Opera and Safari aren't mentioned, etc., but those are second-order concerns. |
CL https://golang.org/cl/12201 mentions this issue. |
When asking pprof for output using
web
, it currently selects browsers using an in-order traversal of the list '"chrome", "google-chrome", "firefox"'. On each platform, it also tries a platform-specific file opener (xdg-open
on *nix) if all the list of browsers all fail to execute.This behavior is non-intuitive, as users' browser preferences are not respect if they have one of those browsers installed, but use another browser by default (this is particularly common for web developers). Instead, only the platform-specific opener should be used, and only in the case where it fails should fallbacks be attempted.
The text was updated successfully, but these errors were encountered: