Open joshka opened 1 month ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 94.2%. Comparing base (
8061813
) to head (f99c224
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
This is mainly an experiment. In theory Widget possibly should be Implemented for FnOnce, but that runs into a conflict with the blanket implementation of Widget for &W: WidgetRef. (Can't implement FnOnce for WidgetRef as the self parameter is a ref.)
Just tossing out my thoughts as a new rust and ratatui user...
I think it would more natural to add a method to Frame
that accepts a FnOnce(&mut Buffer)
. That would be what I'd look for and expect to find if I wanted to use arbitrary code as a rendering function. Treating a function as a Widget
or StatefulWidget
feels complex and unexpected.
fn hello(area: Rect, buf: &mut Buffer) {
Line::raw("Hello").render(area, buf);
}
frame.render(|buf| hello(frame.size(), buf));
I think the FnOnce
should take only the buffer. Why? Seems like Frame::render*
functions just pass the area Rect
through, so the caller can capture it in a closure instead. No need to pass it through the render
call. Since the call site doesn't have a Widget
hand it is likely it will need to pass additional arguments through a closure anyway. So, the more realistic example would be:
fn hello(what: &str, area: Rect, buf:& mut Buffer) {
Line::raw(fmt!("Hello, {}!", what).render(area, buf);
}
frame.render(|buf| hello("World", frame.size(), buf));
At that point, the Frame's render functions accepting Widget
and StatefulWidget
could be implemented in terms of this new and hypothetical Frame::render
function:
Side note: it seems like Widget
and StatefulWidget
exist primarily as syntactic sugar for passing a "closure" (in the sense of "a function plus some associated data") to the Frame's render function. The, currently experimental,"XXXWidgetRef" stuff is yet more complexity for use cases for binding the associated state that are, arguably, already gracefully handled by closures. It might be worth considering retaining render_widget
and render_stateful_widget
as convenience (sugar) but ask less common use cases to use a foundational Frame::render(FnOnce(&mut Buffer)
directly.
Side side note: It seems like Frame
doesn't need to provide the render functions, nor do the Widget
traits need to exist. They could all be dropped in favor of a getter for the Frame's buffer, and application code could simply be expressed like this:
widget.render(frame.size(), frame.buffer());
It took me quite a while to realize that all of the Widget/Frame rendering machinery was essentially a wrapper around the above. I don't really see the point of the Widget
trait machinery, but I admit that I'm new here and could easily be missing something.
I think it would more natural to add a method to
Frame
that accepts aFnOnce(&mut Buffer)
. That would be what I'd look for and expect to find if I wanted to use arbitrary code as a rendering function. Treating a function as aWidget
orStatefulWidget
feels complex and unexpected.
I think perhaps this could be worth considering as an additional thing rather than an alternate. The idea of implementing this for Fn(area, buf) comes pretty much from the systems in bevy without as much complexity In the context of not being in a closure, area and buffer are the two absolutely necessary elements to render something at a location in a buffer.
I think the
FnOnce
should take only the buffer.
Makes sense. The frame owns the buffer, but it doesn't own the area, so being FnOnce(Buffer) -> ()
makes sense
The, currently experimental,"XXXWidgetRef" stuff is yet more complexity for use cases for binding the associated state that are, arguably, already gracefully handled by closures. It might be worth considering retaining
render_widget
andrender_stateful_widget
as convenience (sugar) but ask less common use cases to use a foundationalFrame::render(FnOnce(&mut Buffer)
directly.
Can you expand on this? How would the use cases that require widget ref be gracefully handled by closures? For perspective those are:
Vec<Box<WidgetRef>>
) and render all thoseSide side note: It seems like
Frame
doesn't need to provide the render functions, nor do theWidget
traits need to exist. They could all be dropped in favor of a getter for the Frame's buffer, and application code could simply be expressed like this:
Frame.buffer()
was added fairly recently.It took me quite a while to realize that all of the Widget/Frame rendering machinery was essentially a wrapper around the above. I don't really see the point of the
Widget
trait machinery, but I admit that I'm new here and could easily be missing something.
There's a lot of history and context to this that's difficult to summarize neatly. If you're curious though: I'd start with reading https://github.com/ratatui-org/ratatui/discussions/164 Then https://github.com/ratatui-org/ratatui/pull/833 and https://github.com/ratatui-org/ratatui/pull/903 (and all the linked issues on those - there's a mountain of info)
TL;DR:
fn render(buf, area, args)
. Args gets complex beyond one or two thins.Thanks again for such a great response. I read the previous discussions you linked to and have a better grasp of the background and goals.
I don't want to derail this PR with a broad discussion of issues, so after this reply I'll attempt to politely step back and let a more focused discussion continue.
I think it would more natural to add a method to
Frame
that accepts aFnOnce(&mut Buffer)
. That would be what I'd look for and expect to find if I wanted to use arbitrary code as a rendering function. Treating a function as aWidget
orStatefulWidget
feels complex and unexpected.I think perhaps this could be worth considering as an additional thing rather than an alternate.
With the Frame::buffer()
accessor, arguably the function based API I spoke of is not necessary.
The, currently experimental,"XXXWidgetRef" stuff is yet more complexity for use cases for binding the associated state that are, arguably, already gracefully handled by closures. It might be worth considering retaining
render_widget
andrender_stateful_widget
as convenience (sugar) but ask less common use cases to use a foundationalFrame::render(FnOnce(&mut Buffer)
directly.Can you expand on this? How would the use case that require widget ref be gracefully handled by closures? For perspective those are:
Ahh, now I see the "consume self" problem. It is unfortunate.
Widgets are one of the core ideas of ratatui, and exist as a concept that makes the ecosystem of writing custom widgets work.
* Widget was designed from the outset to consume self - this was likely a mistake and it should have accepted &self * This doesn't work for boxed refs - you need a trait that accepts &self instead of self. That makes WidgetRef necessary
I still can't shake the idea that "widget" idea as currently expressed brings with it some accidental complexity. For example, I'm not seeing a lot of metaprogramming need for a widget trait, so why are there four traits (granted, two experimental) to express the idea of a simple function call?
One way it could be simpler...just maybe....
If the receiver of the render calls were flipped, and each Widget implemented a render_on
method, many call sites could look like this:
my_widget.render_on(frame.buffer(), frame.size())
In fact, this might be quite nice in the common case:
terminal.draw(|frame| {
Paragraph::new("Hello Ratatui! (press 'q' to quit)")
.white()
.on_blue()
.render_on(frame.buffer(), frame.area());
})?;
I think this might satisfy the StatefulWidget
use case (because render_on
could be given an additional argument).
I think this might satisfy the Ref
widgets use case, because render_on
is not a trait method but just a convention, and it can accept self
, &self
or &mut self
as appropriate.
This direction is also backward compatible. Both Widget
and StatefulWidget
API could remain for legacy reasons, but render_on
methods could be added to existing widget impls, and newer "renderable objects" could use the render_on
idiom exclusively.
I don't want to derail this PR with a broad discussion of issues, so after this reply I'll attempt to politely step back and let a more focused discussion continue.
No problem. Let's continue if necessary after this at https://forum.ratatui.rs/t/idea-functionwidget-was-thoughts-on-tui-react/59
my_widget.render_on(frame.buffer(), frame.size())
Adding a functionally equivalent method that swaps the order of parameters would be bad for the UX of widgets in general. Giving multiple ways to do the same exact thing is confusing and requires more explanation than the simple widgets are types that render to a location in a buffer. Making this a convention rather than a trait would also be bad IMHO.
All of these options work today already:
fn render_app(frame: &mut Frame) {
// existing code
frame.render_widget(Paragraph::new("Hello World!"), frame.size());
// ignoring render_widget
Paragraph::new("Hello World!").render(frame.size(), frame.buffer_mut());
// renderging in a function
hello("world!", frame.size(), frame.buffer_mut());
// returning a widget from a function
hello2("world!").render(frame.size(), frame.buffer_mut());
}
fn hello(name: &str, area: Rect, buffer: &mut Buffer) {
Paragraph::new(format!("Hello, {}!", name)).render(area, buffer);
}
fn hello2(name: &str) -> Paragraph {
Paragraph::new(format!("Hello, {}!", name))
}
Using a convention approach doesn't work for boxed widgets. Boxed widgets enables a bunch of future functionality, like to being able to define containers e.g. https://docs.rs/ratatui-widgets/latest/ratatui_widgets/stack_container/struct.StackContainer.html, and creating lists that accept any widget as the content instead of just Text
. It enables things like being able to define tabs or panels that can be inserted / removed from a layout.
This allows you to treat any function that takes a
Rect
and a mutable reference aBuffer
as a widget in situations where you don't want to create a new type for a widget.Example:
Related to: https://forum.ratatui.rs/t/idea-functionwidget-was-thoughts-on-tui-react/59/2