feat: allow configuring render output and stdout/stderr handles#157
feat: allow configuring render output and stdout/stderr handles#157sandydoo wants to merge 6 commits intoccbrown:mainfrom
Conversation
Example usage: ``` element.render_loop() .output(Output::Stderr) .stdout(custom_writer) .stderr(custom_writer) .await ```
| // Check if we have a terminal - if not, messages stay queued | ||
| if updater.terminal_config().is_none() { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The hook messages are no longer printed outside of the render loop, since we don't have a terminal instance in that case.
These messages are side-effects of the render, so skipping over them seems to make sense.
packages/iocraft/src/element.rs
Outdated
| /// Set the stdout handle for hook output. | ||
| /// | ||
| /// Default: `std::io::stdout()` | ||
| pub fn stdout<W: Write + Send + 'static>(mut self, writer: W) -> Self { |
There was a problem hiding this comment.
In theory I think it should be possible to eliminate the 'static requirement.
If it ends up being too ugly to implement the required lifetimes, that's fine, but I don't see any reason it shouldn't be possible for users to do something like...
foo.render_loop().stdout(&w).await;There was a problem hiding this comment.
I gave this a try. If the handles live on SystemContext, then we have to do a transmute to make the compiler happy.
There was a problem hiding this comment.
Yeah, I see now that it would be extremely difficult to give SystemContext (or any context) a lifetime since Any requires 'static.
Unfortunately what you have here I believe isn't quite sound: A user could clone hooks.use_context::<SystemContext>().stdout() and then keep it around past its lifetime.
However, if we pass the writers around in a slightly different way, it's possible to do this soundly and without any unsafe code (and without Arc or Mutex).
This patch (based off of main/a3385b2fbcb7424a3a98f403e82e49dc562d3eea) demonstrates the idea I think. I haven't tested it, but it type-checks at least.
diff --git a/packages/iocraft/src/context.rs b/packages/iocraft/src/context.rs
index a8f0a88..141e50a 100644
--- a/packages/iocraft/src/context.rs
+++ b/packages/iocraft/src/context.rs
@@ -3,6 +3,7 @@ use core::{
cell::{Ref, RefCell, RefMut},
mem,
};
+use std::io::Write;
/// The system context, which is always available to all components.
pub struct SystemContext {
@@ -25,6 +26,30 @@ impl SystemContext {
}
}
+/// Ideally this would be part of `SystemContext`, but since since context relies on `Any` and
+/// `Any` requires `'static`, we can't use regular context for objects with lifetimes. So we'll
+/// pass this around separately alongside the other context.
+pub(crate) struct SystemContextInternal<'a> {
+ pub stdout: Box<dyn Write + Send + Sync + 'a>,
+ pub stderr: Box<dyn Write + Send + Sync + 'a>,
+}
+
+impl<'a> SystemContextInternal<'a> {
+ pub fn new(
+ stdout: Box<dyn Write + Send + Sync + 'a>,
+ stderr: Box<dyn Write + Send + Sync + 'a>,
+ ) -> Self {
+ Self { stdout, stderr }
+ }
+
+ pub fn reborrow(&mut self) -> SystemContextInternal<'_> {
+ SystemContextInternal {
+ stdout: Box::new(&mut *self.stdout),
+ stderr: Box::new(&mut *self.stderr),
+ }
+ }
+}
+
/// A context that can be passed to components.
pub enum Context<'a> {
/// Provides the context via a mutable reference. Children will be able to get mutable or
@@ -87,13 +112,18 @@ impl<'a> Context<'a> {
#[doc(hidden)]
pub struct ContextStack<'a> {
+ system_context_internal: SystemContextInternal<'a>,
contexts: Vec<RefCell<Context<'a>>>,
}
impl<'a> ContextStack<'a> {
- pub(crate) fn root(root_context: &'a mut (dyn Any + Send + Sync)) -> Self {
+ pub(crate) fn root(
+ system_context: &'a SystemContext,
+ system_context_internal: SystemContextInternal<'a>,
+ ) -> Self {
Self {
- contexts: vec![RefCell::new(Context::Mut(root_context))],
+ system_context_internal,
+ contexts: vec![RefCell::new(Context::from_ref(system_context))],
}
}
@@ -139,4 +169,8 @@ impl<'a> ContextStack<'a> {
}
None
}
+
+ pub(crate) fn system_context_internal_mut(&mut self) -> &mut SystemContextInternal<'a> {
+ &mut self.system_context_internal
+ }
}
diff --git a/packages/iocraft/src/element.rs b/packages/iocraft/src/element.rs
index d35c8fa..7a0838f 100644
--- a/packages/iocraft/src/element.rs
+++ b/packages/iocraft/src/element.rs
@@ -289,6 +289,8 @@ enum RenderLoopFutureState<'a, E: ElementExt> {
Init {
fullscreen: bool,
ignore_ctrl_c: bool,
+ stdout: Box<dyn Write + Send + Sync + 'a>,
+ stderr: Box<dyn Write + Send + Sync + 'a>,
element: &'a mut E,
},
Running(Pin<Box<dyn Future<Output = io::Result<()>> + Send + 'a>>),
@@ -309,6 +311,8 @@ impl<'a, E: ElementExt + 'a> RenderLoopFuture<'a, E> {
state: RenderLoopFutureState::Init {
fullscreen: false,
ignore_ctrl_c: false,
+ stdout: Box::new(stdout()),
+ stderr: Box::new(stderr()),
element,
},
}
@@ -342,6 +346,26 @@ impl<'a, E: ElementExt + 'a> RenderLoopFuture<'a, E> {
}
self
}
+
+ pub fn stdout<W: Write + Send + Sync + 'a>(mut self, stdout: W) -> Self {
+ match &mut self.state {
+ RenderLoopFutureState::Init { stdout: out, .. } => {
+ *out = Box::new(stdout);
+ }
+ _ => panic!("stdout() must be called before polling the future"),
+ }
+ self
+ }
+
+ pub fn stderr<W: Write + Send + Sync + 'a>(mut self, stderr: W) -> Self {
+ match &mut self.state {
+ RenderLoopFutureState::Init { stderr: err, .. } => {
+ *err = Box::new(stderr);
+ }
+ _ => panic!("stderr() must be called before polling the future"),
+ }
+ self
+ }
}
impl<'a, E: ElementExt + Send + 'a> Future for RenderLoopFuture<'a, E> {
@@ -354,13 +378,15 @@ impl<'a, E: ElementExt + Send + 'a> Future for RenderLoopFuture<'a, E> {
loop {
match &mut self.state {
RenderLoopFutureState::Init { .. } => {
- let (fullscreen, ignore_ctrl_c, element) =
+ let (fullscreen, ignore_ctrl_c, element, stdout, stderr) =
match std::mem::replace(&mut self.state, RenderLoopFutureState::Empty) {
RenderLoopFutureState::Init {
fullscreen,
ignore_ctrl_c,
element,
- } => (fullscreen, ignore_ctrl_c, element),
+ stdout,
+ stderr,
+ } => (fullscreen, ignore_ctrl_c, element, stdout, stderr),
_ => unreachable!(),
};
let mut terminal = match if fullscreen {
@@ -374,7 +400,7 @@ impl<'a, E: ElementExt + Send + 'a> Future for RenderLoopFuture<'a, E> {
if ignore_ctrl_c {
terminal.ignore_ctrl_c();
}
- let fut = Box::pin(terminal_render_loop(element, terminal));
+ let fut = Box::pin(terminal_render_loop(element, terminal, stdout, stderr));
self.state = RenderLoopFutureState::Running(fut);
}
RenderLoopFutureState::Running(fut) => {
diff --git a/packages/iocraft/src/hooks/use_output.rs b/packages/iocraft/src/hooks/use_output.rs
index 074d6fd..119f003 100644
--- a/packages/iocraft/src/hooks/use_output.rs
+++ b/packages/iocraft/src/hooks/use_output.rs
@@ -76,6 +76,13 @@ impl UseOutputState {
return;
}
+ let stdout = &mut updater
+ .component_context_stack_mut()
+ .system_context_internal_mut()
+ .stdout;
+ stdout.write(b"foo\n").unwrap();
+ // TODO: actually use this and the configured stderr for the output
+
updater.clear_terminal_output();
if let Some(col) = self.appended_newline {
let _ = queue!(std::io::stdout(), cursor::MoveUp(1), cursor::MoveRight(col));
diff --git a/packages/iocraft/src/render.rs b/packages/iocraft/src/render.rs
index 9f4813e..8552afa 100644
--- a/packages/iocraft/src/render.rs
+++ b/packages/iocraft/src/render.rs
@@ -1,7 +1,7 @@
use crate::{
canvas::{Canvas, CanvasSubviewMut},
component::{ComponentHelperExt, Components, InstantiatedComponent},
- context::{Context, ContextStack, SystemContext},
+ context::{Context, ContextStack, SystemContext, SystemContextInternal},
element::ElementExt,
multimap::AppendOnlyMultimap,
props::AnyProps,
@@ -17,7 +17,7 @@ use futures::{
future::{select, FutureExt, LocalBoxFuture},
stream::{Stream, StreamExt},
};
-use std::io;
+use std::io::{self, Write};
use taffy::{
AvailableSpace, Display, Layout, NodeId, Overflow, Point, Rect, Size, Style, TaffyTree,
};
@@ -88,6 +88,11 @@ impl<'a, 'b, 'c> ComponentUpdater<'a, 'b, 'c> {
self.component_context_stack
}
+ #[doc(hidden)]
+ pub fn component_context_stack_mut(&mut self) -> &mut ContextStack<'c> {
+ self.component_context_stack
+ }
+
/// Gets an immutable reference to context of the given type.
pub fn get_context<T: Any>(&self) -> Option<Ref<T>> {
self.component_context_stack.get_context()
@@ -347,6 +352,7 @@ struct Tree<'a> {
root_component: InstantiatedComponent,
root_component_props: AnyProps<'a>,
system_context: SystemContext,
+ system_context_internal: SystemContextInternal<'a>,
}
struct RenderOutput {
@@ -355,7 +361,12 @@ struct RenderOutput {
}
impl<'a> Tree<'a> {
- fn new(mut props: AnyProps<'a>, helper: Box<dyn ComponentHelperExt>) -> Self {
+ fn new(
+ mut props: AnyProps<'a>,
+ helper: Box<dyn ComponentHelperExt>,
+ stdout: Box<dyn Write + Send + Sync + 'a>,
+ stderr: Box<dyn Write + Send + Sync + 'a>,
+ ) -> Self {
let mut layout_engine = TaffyTree::new();
let root_node_id = layout_engine
.new_leaf_with_context(Style::default(), LayoutEngineNodeContext::default())
@@ -369,6 +380,7 @@ impl<'a> Tree<'a> {
root_component: InstantiatedComponent::new(root_node_id, props.borrow(), helper),
root_component_props: props,
system_context: SystemContext::new(),
+ system_context_internal: SystemContextInternal::new(stdout, stderr),
}
}
@@ -384,7 +396,10 @@ impl<'a> Tree<'a> {
layout_engine: &mut self.layout_engine,
did_clear_terminal_output: false,
};
- let mut component_context_stack = ContextStack::root(&mut self.system_context);
+ let mut component_context_stack = ContextStack::root(
+ &self.system_context,
+ self.system_context_internal.reborrow(),
+ );
self.root_component.update(
&mut context,
&mut wrapper_child_node_ids,
@@ -486,16 +501,26 @@ impl<'a> Tree<'a> {
pub(crate) fn render<E: ElementExt>(mut e: E, max_width: Option<usize>) -> Canvas {
let h = e.helper();
- let mut tree = Tree::new(e.props_mut(), h);
+ let mut tree = Tree::new(
+ e.props_mut(),
+ h,
+ Box::new(io::stdout()),
+ Box::new(io::stderr()),
+ );
tree.render(max_width, None).canvas
}
-pub(crate) async fn terminal_render_loop<E>(e: &mut E, term: Terminal) -> io::Result<()>
+pub(crate) async fn terminal_render_loop<E>(
+ e: &mut E,
+ term: Terminal,
+ stdout: Box<dyn Write + Send + Sync + '_>,
+ stderr: Box<dyn Write + Send + Sync + '_>,
+) -> io::Result<()>
where
E: ElementExt,
{
let h = e.helper();
- let mut tree = Tree::new(e.props_mut(), h);
+ let mut tree = Tree::new(e.props_mut(), h, stdout, stderr);
tree.terminal_render_loop(term).await
}
@@ -528,7 +553,8 @@ where
{
let (term, output) = Terminal::mock(config);
MockTerminalRenderLoop {
- render_loop: terminal_render_loop(e, term).boxed_local(),
+ render_loop: terminal_render_loop(e, term, Box::new(io::stdout()), Box::new(io::stderr()))
+ .boxed_local(),
render_loop_is_done: false,
output,
}
@@ -604,7 +630,13 @@ mod tests {
#[apply(test!)]
async fn test_terminal_render_loop_send() {
let (term, _output) = Terminal::mock(MockTerminalConfig::default());
- await_send_future(terminal_render_loop(&mut element!(MyComponent), term)).await;
+ await_send_future(terminal_render_loop(
+ &mut element!(MyComponent),
+ term,
+ Box::new(io::stdout()),
+ Box::new(io::stderr()),
+ ))
+ .await;
}
#[component]There was a problem hiding this comment.
Unfortunately what you have here I believe isn't quite sound: A user could clone hooks.use_context::().stdout() and then keep it around past its lifetime.
Ah, good point!
However, if we pass the writers around in a slightly different way, it's possible to do this soundly and without any unsafe code (and without Arc or Mutex).
SystemContextInternal would work, but we'd still need an Arc + Mutex to share the handle with StdTerminal. So we're left with another ownership conflict: both the terminal and the output hooks want to write to the same handles.
Which got me thinking, why don't we store both handles on StdTerminal? Then the output hooks borrow them to write their messages.
This also avoids having to drill the handles through Tree, which feels off to me.
I've pushed two commits:
9fea6ac to
f4f8138
Compare
|
crossterm's keyboard enhancement check writes to stdout as fallback: https://github.com/crossterm-rs/crossterm/blob/4f08595ef4477de2d504dcced24060ed9e3d582a/src/terminal/sys/unix.rs#L237-L241 I'll open an issue upstream. We can add a guard against this in the meantime. |
0034652 to
e9c5096
Compare
Both StdTerminal and output hooks need to write to the handles. We have two options: 1. use Arc + Mutex to share the handles and control write access 2. make the terminal own the handles and provide write access via methods This implements option 2.
e9c5096 to
1215852
Compare
ccbrown
left a comment
There was a problem hiding this comment.
The general approach and the API lgtm!
I'm just trying to convince myself that all of the writes are happening in the correct place. I think the flipping that Terminal does makes it kind of hard to follow.
To test this out, I tried making a simple modification to the use_output example:
diff --git a/examples/use_output.rs b/examples/use_output.rs
index 4a968e7..6506889 100644
--- a/examples/use_output.rs
+++ b/examples/use_output.rs
@@ -25,5 +25,5 @@ fn Example(mut hooks: Hooks) -> impl Into<AnyElement<'static>> {
}
fn main() {
- smol::block_on(element!(Example).render_loop()).unwrap();
+ smol::block_on(element!(Example).render_loop().output(Output::Stderr)).unwrap();
}I expected the output to look the same, but somehow the output was corrupted:
Any idea why this didn't work?
One option would be to call them something like
This should now work. The last blockers here have to do with crossterm. For example, cursor position is hardcoded to use stdout. This is fine when both stdout and stderr are TTYs, but breaks when stdout is piped. Related issue: crossterm-rs/crossterm#652 |
That would probably be at least a little bit better.
Hmm... unfortunately it doesn't look like crossterm is actively reviewing and merging PRs. And it looks like crossterm is using some private, global state internally, so we can't just use modified portions of the relevant code from it. Any ideas other than forking the entire crate? |
Yeah, the situation with crossterm is a little unfortunate. We'll continue pushing the necessary upstream fixes. As for alternatives, termion seems to have the better API for cursor positioning that doesn't hardcode stdout. No support for Windows though. |
What It Does
Adds the following render loop methods:
.stdout(writer): sets the handle to use when writing to stdout..stderr(writer): sets the handle to use when writing to stderr..output(<Output>): configures which handle to use to render to: stderr or stdout.The
use_outputhooks also take into account the above options.Related Issues
Fixes #156.