Revert conditional setting of stdin handle #3379
Conversation
the-mikedavis
changed the title
|
Switched to conditional compilation and tested on Windows and Ubuntu (WSL). Everything seemed to work as intended. |
helix-term/src/commands.rs
Outdated
| #[cfg(not(target_family = "windows"))] | ||
| if input.is_some() { | ||
| process.stdin(Stdio::piped()); | ||
| } | ||
| #[cfg(target_family = "windows")] | ||
| { | ||
| process.stdin(Stdio::piped()); | ||
| } |
There was a problem hiding this comment.
This doesn't need conditional compiling since it compiles ok on and off windows. The runtime behavior should be influenced by it though and that can be checked with the cfg! macro
if input.is_some() || cfg!(windows) {
precss.stdin(Stdio::piped());
}There was a problem hiding this comment.
Is this usually preferable?
There was a problem hiding this comment.
From what I understand, it's best to only use what conditional compilation you really need. That way you don't mistakenly end up with wildly different code-paths on different operating systems. At the very least it makes rust-analyzer a little quieter to keep conditional compiles to a minimum :P
There was a problem hiding this comment.
Gotcha, gotcha. Thanks!
I did have an issue with my first attempt, even though I was pretty sure it wasn't going to work. It did compile, but the binary would just hang there. The most surprising thing is that this was legal, and it compiled.
#[cfg(target_family = "windows")]
let a = "hello";
* Revert 3121353 * Switch to conditional compilation * Run formatter * Switch from conditional compilation to compile-time bool
* Revert 3121353 * Switch to conditional compilation * Run formatter * Switch from conditional compilation to compile-time bool
fixes #3374
I still like the change to avoid setting stdio if necessary, but it looks like this causes an issue on Windows.
After attempting other fixes, reverting the changes was the only thing that worked.
P.S. Conditional compilation could be added for non-Windows targets, but I think this would not add value to the codebase or Helix.