feat(windows): use std Command

[matoous]

Use std Command instead of ShellExecuteW from windows sys crate.

This change was already attempted in: #25 and later reverted in: #27 and it it seems that it didn't work due to incorrect usage of explorer instead of cmd /c start.
(see helix-editor/helix#5820 (comment) for detailed explanation).

Related: helix-editor/helix#5820

---

• test this on windows

[Byron]

I am currently taking a look at the build failures - I should be able to get the fix here quickly due to access to a Windows VM.

[Byron]

Thanks a lot for your help!

I am looking forward to the that_commands(…) implementation that will enable tokio interop in case you would like to try that too.

[matoous]

@Byron I gave it a shot here: #64, happy for any feedback.

[wez]

This now causes cmd windows to pop up. Is that intentional?

[Byron]

Thanks for letting me know, I wasn't aware. Thinking about it, my testing was done from within a terminal application, which didn't pop up another terminal so I thought it was alright - wishful thinking. As a little retrospective, a couple of folks looked into why Windows was so complicated and it wasn't obvious anymore and thus seems like a good idea to make it uniform to support the helix-editor.

It looks like v3.2 is the last version that uses the windows crate based mechanism and thus shouldn't open a terminal, and I yanked v3.3 that you might have upgraded to. This leaves v4.0 with the breaking change.

I think it will be yanked as well and replaced with a new crate called open-te, an open for terminal applications which should be used by helix-editor instead. Maintaining two versions of the crate in this repo seems manageable.

@pascalkuthe as open-te would be a new crate which then would be specifically done for helix I wonder if this is something helix wants to have control over. If so, I will yank v4.0 and leave it all to you while rolling back this repo to an older version. Please let me know your preference so I know which action to take.

Maybe there are other options I am not seeing, if so please let me know as well.

[pascalkuthe]

We will eventually create a GUI for helix too so it would be good to have a proper fix. I think adding the /min flag to start should be enoug but I can't test on windows myself right now: cmd /c start /min

[wez]

FWIW, while I understand the appeal of the uniformity of cmd /c start, it is still not "as good" as using ShellExecute; Windows really doesn't like subprocesses and they introduce a number of weird issues to contend with, and are generally slow to create.

If you are committed to running a subprocess for this, you should look into:
https://doc.rust-lang.org/std/os/windows/process/trait.CommandExt.html#tymethod.creation_flags
and pass in CREATE_NO_WINDOW to prevent a console window from being shown by the system.

[pascalkuthe]

FWIW, while I understand the appeal of the uniformity of cmd /c start, it is still not "as good" as using ShellExecute; Windows really doesn't like subprocesses and they introduce a number of weird issues to contend with, and are generally slow to create.

If you are committed to running a subprocess for this, you should look into: https://doc.rust-lang.org/std/os/windows/process/trait.CommandExt.html#tymethod.creation_flags and pass in CREATE_NO_WINDOW to prevent a console window from being shown by the system.

the problem is that we want to use the commands returned by open-rs in an async context where blocking is not acceptable by returning an array of std Command we can convert to tokio Commands and then await those. I am not aware of a different simple solution to that problem. Thanks for the CREATE_NO_WINDOW pointer thtat should work 👍

[wez]

I'm not sure if tokio's process spawning will work with creation_flags. I can see that it can convert from the std Command to the tokio equivalent, but I don't know if that will be enough because the docs for tokio's command suggest that it does some custom spawn handling.

An alternative approach, if that doesn't work, is to use spawn_blocking to adapt the process wait to async code (if you want to sort of retain uniformity), or even just have the ShellExecute call in a spawn_blocking block.

[pascalkuthe]

I also thought about doing spawn_blocking but it would be kind of unfortunate to bog every platform down because of windows. It's also not quite the same thing because if you drop the join handle the command will keep running in the background. In theory xdg-open/start can do anything (including spinning forever) so I would be really uncomfortable with just a spawn-blocking something that could spin forever in the background. Tokio commands automatically kill the subprocess if you drop them (and the await can ofcourse be canceled anytime) but canceling the windows API call is basically impossible.

That said tokio uses the std spawning mechanisim on windows (see https://github.com/tokio-rs/tokio/blob/4cd4b02389e663c5e18edbaf1f042882bc271392/tokio/src/process/windows.rs#L70) so at least the creation flag should work.

[Byron]

Thanks @wez and @pascalkuthe for hashing out a solution - it turns out to be working just like you think as per #69 and a new release is available with a fix.