Add Process.output #1235
Add Process.output #1235
Conversation
|
This needs tests. Should there also be a corresponding error function? Is this the way/func-name Go uses? Python has .communicate to wait and return (out, err)... |
|
I've rebased and cleaned up this PR. |
- Allow async functions in REPL (denoland#1233) - Handle Location header relative URI (denoland#1240) - Add deno.readAll() (denoland#1234) - Add Process.output (denoland#1235) - Upgrade to TypeScript 3.2.1 - Upgrade crates: tokio 0.1.13, hyper 0.12.16, ring 0.13.5
| @@ -59,6 +61,21 @@ export class Process { | |||
| return await runStatus(this.rid); | |||
| } | |||
|
|
|||
| /** Buffer the stdout and return it as Uint8Array after EOF. | |||
| * You must have set stdout to "piped" in when creating the process. | |||
| * This calls close() on stdout after its done. | |||
There was a problem hiding this comment.
That's a somewhat questionable design choice IMO... It's begging for resource leaks.
Let's say the process fails ... now the user has to remember to explicitly close the pipe.
There was a problem hiding this comment.
The close is in a try-finally block, so I believe that would actually work. But yes, it's a debatable behavior....Go's cmd.Output does not close the pipe, https://golang.org/pkg/os/exec/#Cmd.Output but in Go you specify the pipe explicitly where as here the system creates the pipe for you...
There was a problem hiding this comment.
We definitely need to make a second pass at the subprocess API.
There was a problem hiding this comment.
The close is in a try-finally block, so I believe that would actually work
That's not what I mean.
The problem is that the user needs to call output() at all, otherwise the handle leaks.
In many cases there'd be no point in calling output() if the process failed.
Depend on #1234
Related #1216