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

Content/build.fsx: use CreateProcess #32

Conversation

dharmatech
Copy link
Contributor

@dharmatech dharmatech commented Feb 13, 2021

Summary of changes

Use CreateProcess instead of Process.start

Issue

If I go through the "How to start" guide for Saturn:

https://saturnframework.org/tutorials/how-to-start.html

when I run the last step:

dotnet fake build -t run

the webserver does indeed appear to start. I can go to http://localhost:8085/books and see the resulting application.

However, if I ctrl-c at the console to stop the webserver, I notice the following:

image

On Windows, the browser is never launched.

Changes

The following line:

    Process.start (fun i -> { i with FileName = "http://localhost:8085" }) |> ignore

was replaced with the following:

    if RuntimeInformation.IsOSPlatform(OSPlatform.Windows) then
      CreateProcess.fromRawCommand "cmd.exe" [ "/C"; "start http://localhost:8085" ] |> Proc.run |> ignore
    elif RuntimeInformation.IsOSPlatform(OSPlatform.Linux) then
      CreateProcess.fromRawCommand "xdg-open" [ "http://localhost:8085" ] |> Proc.run |> ignore
    elif RuntimeInformation.IsOSPlatform(OSPlatform.OSX) then
      CreateProcess.fromRawCommand "open" [ "http://localhost:8085" ] |> Proc.run |> ignore

Notes

Some discussion of the issue on stackoverflow:

https://stackoverflow.com/questions/66139159/process-start-being-used-to-launch-browser-but-is-now-deprecated

Thanks to user nh2 who made the suggestion to take a look at how Plotly.NET launches browsers:

https://stackoverflow.com/a/66184603/268581

Use CreateProcess instead of Process.start
@Krzysztof-Cieslak Krzysztof-Cieslak merged commit 691b671 into SaturnFramework:master Feb 19, 2021
@Krzysztof-Cieslak
Copy link
Member

Thanks a lot!

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.

2 participants