Correct paths windows backslashes and spawn shell param.#112
Correct paths windows backslashes and spawn shell param.#112natefaubion merged 3 commits intoaristanetworks:mainfrom
Conversation
backend-es/src/Main.purs
Outdated
| , Just $ ShareStream (unsafeCoerce Process.stdout) | ||
| , Just $ ShareStream (unsafeCoerce Process.stderr) | ||
| -- To preserve colors in stdout need to pass it into spawn's stdio. | ||
| childProc <- unsafeCoerce <$> UnsafeChildProcess.spawn' command args |
There was a problem hiding this comment.
Why is the unsafeCoerce needed here? Presumably the map is unnecessary?
There was a problem hiding this comment.
Because below used ChildProcess.stdin and other functions for safe ChildProcess, if to use unsafeStdin it returns Nullable Writtable, it should be handled, map can be ommited yes.
There was a problem hiding this comment.
If you suppose it is more correct not ot use unsafeCoerce here, I can fix this too.
backend-es/src/Main.purs
Outdated
| Process.exit' 1 | ||
| pure $ effectCanceler do | ||
| ChildProcess.kill SIGABRT childProc | ||
| void $ ChildProcess.kill' (stringSignal "SIGABRT") childProc |
There was a problem hiding this comment.
Can this use https://pursuit.purescript.org/packages/purescript-node-child-process/11.1.0/docs/Node.ChildProcess#v:killSignal to avoid the string? These functions are largely undocumented so I'm not really sure what the difference is.
There was a problem hiding this comment.
Missed this functions. Seems it it does the same just converts Signal to string and passes it to kill'.
backend-es/test/Utils.purs
Outdated
| Process.exit' 1 | ||
| pure $ effectCanceler do | ||
| ChildProcess.kill SIGABRT childProc | ||
| void $ ChildProcess.kill' (stringSignal "SIGABRT") childProc |
There was a problem hiding this comment.
Same question here regarding killSignal.
backend-es/test/Utils.purs
Outdated
| pure $ effectCanceler $ ChildProcess.kill SIGABRT childProc | ||
| childProc <- ChildProcess.exec' command identity (k <<< pure) | ||
| _ <- Stream.writeString' (ChildProcess.stdin childProc) UTF8 input mempty | ||
| --Stream.end (ChildProcess.stdin childProc) mempty |
There was a problem hiding this comment.
Should this comment be removed?
backend-es/test/Utils.purs
Outdated
| _ <- Stream.writeString' (ChildProcess.stdin childProc) UTF8 input mempty | ||
| --Stream.end (ChildProcess.stdin childProc) mempty | ||
| (ChildProcess.stdin childProc) # on_ finishH mempty | ||
| pure $ effectCanceler $ void $ ChildProcess.kill' (stringSignal "SIGABRT") childProc |
|
Since you bumped the package set, we need to change the CI script: |
|
Thanks! |
2 participants
This fixes running the package on Windows.
Should fix this: #104