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 launbrowser on run #190

Merged
merged 4 commits into from
Sep 9, 2020
Merged

add launbrowser on run #190

merged 4 commits into from
Sep 9, 2020

Conversation

mgautam98
Copy link
Contributor

Towards #179
Opens the default browser with the specified port.

Verified and tested on:

  1. Windows 10 Pro
  2. WSL2 (Distro: Ubuntu 18.04) on Windows
  3. Debian (Kali Linux)

src/webserver/WebServer.jl Outdated Show resolved Hide resolved
@fonsp
Copy link
Owner

fonsp commented Jun 15, 2020

And can you put everything in a try catch block?

@mgautam98
Copy link
Contributor Author

mgautam98 commented Jun 16, 2020

Hey I found that on WSL1 we can not open Host system browser(Or any application, On WSL2 we can) I am trying to find a workaround this.

@mgautam98
Copy link
Contributor Author

@fonsp can you check now

@fonsp
Copy link
Owner

fonsp commented Jun 25, 2020

Looking good! Just some style comments:

  • the same command can be used for the Windows case and the WSL case
  • the error message should go
  • Julia has a built-in way to check for WSL

@radonnachie
Copy link

I'm keen to help finish this off, only confused about @fonsp
Julia has a built-in way to check for WSL
I cannot find related information...

I also don't see an issue with leaving the error message? Just curious about what it was requested to go, won't argue <3

@mgautam98
Copy link
Contributor Author

I'm keen to help finish this off, only confused about @fonsp
Julia has a built-in way to check for WSL
I cannot find related information...

I also don't see an issue with leaving the error message? Just curious about what it was requested to go, won't argue <3

There is currently no built in way to do this. But I opened a PR in JuliaLang too for this, JuliaLang/julia#36425

All the work is already done I don't think there is anything to complete @RocketRoss

@radonnachie
Copy link

radonnachie commented Aug 16, 2020

Yeah I see that. Nice!
Could you share the julialang request?🤦
I just want it to be merged. Keen to have the observable notebook environments. So even if my comment just serves to relight the fire (:

@fonsp
Copy link
Owner

fonsp commented Aug 16, 2020

Ah you're right, not sure where I got that from.

I still have these comments but I can also do them myself sometime?

@fonsp
Copy link
Owner

fonsp commented Sep 3, 2020

Sorry @mgautam98 I'll merge this soon!

@fonsp fonsp merged commit d0ff39e into fonsp:master Sep 9, 2020
@fonsp
Copy link
Owner

fonsp commented Sep 9, 2020

Thanks @mgautam98!

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.

3 participants