Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callback streams for use as library #2799

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

bakaq
Copy link
Contributor

@bakaq bakaq commented Jan 29, 2025

This adds a new StreamConfig option that allows both to asynchronously react to the Machine writing to stdout or stderr, and to programatically send data to its stdin, all in-memory.

This is an incredible feature for many reasons, like being able to get stdout in parts and not just all at once, but the application I have in mind is running the same toplevel we use for CLI inside Wasm in the Scryer Playground, which will be a great improvement in usability!

@Skgland
Copy link
Contributor

Skgland commented Jan 29, 2025

Rather than having one value in StreamConfig that configures stdin/stdout/stderr at once would it maybe make sense to have individual configs per stream like what I currently have in 6d0ddc1 as part of #2786.

Also (unrelated to this PR) having in_memory configure the stdin to be a null stream, stdout to be buffered in memory and stderr to still be stderr is a bit confusing.

src/machine/config.rs Outdated Show resolved Hide resolved
src/machine/config.rs Outdated Show resolved Hide resolved
@bakaq
Copy link
Contributor Author

bakaq commented Jan 30, 2025

I guess it also makes sense to make OutputStreamConfig and InputStreamConfig public and allow some builder style configuration in StreamConfig (like with_stdin, with_stdout and with_stderr methods, although I think these particular names are confusing), and also fix the issue you pointed out with in_memory, but I'm not sure this should happen in this PR.

@jjtolton
Copy link

👀 what's the high level of this? configurable input/output/error streams? If so, I'm all for it! If not... probably still all for it, just not sure what I'm all for yet!

@bakaq
Copy link
Contributor Author

bakaq commented Jan 30, 2025

Yes, this lets you handle stdin, stdout and stderr programatically, instead of it leaking to the real stdin, stdout or stderr.

@jjtolton
Copy link

Yes, this lets you handle stdin, stdout and stderr programatically, instead of it leaking to the real stdin, stdout or stderr.

I assume this does not apply to shell/N? different PR, probably?

@bakaq
Copy link
Contributor Author

bakaq commented Jan 30, 2025

Yeah, shell/N would be Scryer controlling another process. This PR is concerned with the embedding API, and specifically with being able to control Scryer itself, so almost the exact opposite.

@bakaq
Copy link
Contributor Author

bakaq commented Feb 1, 2025

I removed the way to make input null streams too. Now the default is an empty memory buffer. I also made some more tests, and in the process added support for converting streams to Terms, something that was missing from the work on #2475. I believe null streams should eventually be changed back to the default as soon as they have sane behavior (at least not crashing or panicking on use).

Something funny I noticed is that write/1 flushes the output but nl/0 doesn't. I would expect the exact opposite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants