Add --read-logs-from for asynchronous log output
#227
Conversation
9999years
force-pushed
the
rebeccat/dux-2225-read-logs-from-file
branch
from
794954c
to
ba003e7
Compare
| // TODO: Lock stdout here and for ghci output. | ||
| let _ = stdout.write_all(line.as_bytes()).await; | ||
| let _ = stdout.write_all(b"\n").await; |
There was a problem hiding this comment.
This is a pretty big "TODO". std::io::Stdout is globally synchronized, but tokio::io::Stdout is not, so this could lead to interleaved output.
AFAICT there's no easy solution for this. The tokio devs suggested creating a task to manage stdout and sending it messages (strings to write) over a channel, which is reasonable enough, but then I'll have to write an adapter to implement std::io::Write by sending messages to an async channel.
| ReadLogsFrom { | ||
| shutdown: handle, | ||
| path, | ||
| } | ||
| .run() |
There was a problem hiding this comment.
This is not a blocker, but it wasn't clear to me why this is split up as a struct and a method on that struct (i.e. ReadLogsFrom { shutdown: handle, path }.run()) and not combined into a single function (i.e. readLogsFrom(handle, path))
There was a problem hiding this comment.
A couple reasons why I chose to implement it like this:
- Struct+method is a lot easier to split up to deal with complex logic.
- Methods don't have named parameters, so when there's a lot of state to pass in, using a struct is a lot clearer (and lower-overhead than defining builders).
- Most of the async tasks in
ghciwatchare structured like this.
Because ghciwatch reads ghci's output and expects it to be structured in a standard way, it can't cope with programs that output to stdout (or stderr) asynchronously (see ndmitchell/ghcid#137).
To make this easier for frameworks that output logs (like Yesod), we should have an option to read logs asynchronously from a file. If provided, writes to the given file will show up in ghciwatch's output (but not ghci's output).