jlfwong / speedscope

🔬 A fast, interactive web-based viewer for performance profiles.
https://www.speedscope.app
MIT License
5.52k stars 241 forks source link

Doesn't open browser in Windows #168

Open janpio opened 6 years ago

janpio commented 6 years ago
C:\Projects\Fastlane\optimize_startup_time (master -> origin)
λ speedscope C:\Users\Jan\.cache\rbspy\records\rbspy-2018-09-27-Y7AYEI5ihd.speedscope.json
Creating temp file C:\Users\Jan\AppData\Local\Temp\speedscope-1538089620601-55152.js
Creating temp file C:\Users\Jan\AppData\Local\Temp\speedscope-1538089620601-55152.html
Opening file://C:\Users\Jan\AppData\Local\Temp\speedscope-1538089620601-55152.html in your default browser

Nothing happens.

Opening file://C:\Users\Jan\AppData\Local\Temp\speedscope-1538089620601-55152.html manually worked.

jlfwong commented 6 years ago

Hi @janpio, thanks for reporting this! I've done verrry close to zero testing on windows, so this doesn't particularly surprise me :)

I'll try to find some time to investigate this on a Windows VM.

Out of curiousity, which shell are running speedscope in on Windows? Is this powershell or git bash or something else?

janpio commented 6 years ago

Yeah, it didn't really surprise me as well, so no harm done ;) Easiest solution is probably to change the text on Windows that the user should open it manually.

I was using Cmder right now, that usually behaves a bite more like git bash or a macOS or Linux shell than Powershell or cmd.exe. But should work everywhere of course 🔫

spillerrec commented 4 years ago

Changing the line cli.js:92:

  await opn(urlToOpen, {wait: false})

to wait: true solves the issue for me both in cmd and Powershell. I also notice that the 'opn' package has been renamed to 'open' and the old package has been deprecated. Using 'opn' or 'open' does not make any difference as far as I can tell.

jlfwong commented 4 years ago

@spillerrec Oh sweet, thanks for figuring that out. I'd accept a PR to switch packages & patch the bug if you're interested!

spillerrec commented 4 years ago

I have created a PR here: https://github.com/jlfwong/speedscope/pull/307 I haven't worked on Node projects before, so let me know if I managed to mess it up despite the small amount of changes.

jlfwong commented 4 years ago

This should be fixed now on master thanks to @spillerrec, and will go out with the next release. No promises on when that's going to be, but hopefully sometime in the next week or two.

jlfwong commented 4 years ago

This has now been released, so it should be fixed in speedscope@1.10.0.