Rename render_ref for StatefulWidgetRef to render_stateful_ref

[kdheepak]

Problem

Currently both WidgetRef and StatefulWidgetRef use render_ref method names.

ratatui/src/widgets.rs

Lines 298 to 303 in 8719608

	#[stability::unstable(feature = "widget-ref")]
	pub trait WidgetRef {
	/// Draws the current state of the widget in the given buffer. That is the only method required
	/// to implement a custom widget.
	fn render_ref(&self, area: Rect, buf: &mut Buffer);
	}

ratatui/src/widgets.rs

Lines 395 to 404 in 8719608

	#[stability::unstable(feature = "widget-ref")]
	pub trait StatefulWidgetRef {
	/// State associated with the stateful widget.
	///
	/// If you don't need this then you probably want to implement [`WidgetRef`] instead.
	type State;
	/// Draws the current state of the widget in the given buffer. That is the only method required
	/// to implement a custom stateful widget.
	fn render_ref(&self, area: Rect, buf: &mut Buffer, state: &mut Self::State);
	}

If a user imports both traits, then they have to explicitly call the method using the trait name, i.e. they have to write StatefulWidgetRef::render_ref(widget, area, buf, state) instead of widget.render_ref(area, buf, state).

Renaming render_ref to render_stateful_ref will prevent the ambiguity between WidgetRef and StatefulWidgetRef.

It would be nice to disambiguate it for Widget and StatefulWidget as well but that would be a big breaking change. For WidgetRef and StatefulWidgetRef, these traits are still unstable at the moment so we can make this change.

[joshka]

+1 on this. I think while it's inconsistent with the StatefulWidget, it's the right choice.

One alternative worth considering is naming the traits: RenderRef and RenderRefWithState and creating two new traits to cover the old widget ones. I'm not sure how I feel about that idea however.

[sww1235]

It would be nice to disambiguate it for Widget and StatefulWidget as well but that would be a big breaking change. For WidgetRef and StatefulWidgetRef, these traits are still unstable at the moment so we can make this change.

The Widget/StatefulWidget part just bit me when implementing Widget as opposed to using one big layout function.
It would be nice to fix this API issue in future.

[kdheepak]

Maybe:

• RenderRef
• RenderRefWithState
• RenderOwned
• RenderOwnedWithState

? And at some point in the future we can deprecate Widget and StatefulWidget.

[joshka]

And at some point in the future we can deprecate Widget and StatefulWidget.

The hassle is that doing this breaks everything conceptually (as well as semver). The pay off seems low as it's easy to work around.

[EdJoPaTo]

Im not sure that we actually need RenderRef and RenderOwned. We just need one and implement it either for the &ListWidget or for ListWidget (impl Render for &ListWidget). That way its the same trait and the widget knows whether it requires ownership on rendering.

[joshka]

A trait that accepts &self is required in order to make boxed traits work.

[joshka]

(Removed "good first issue" label while we're discussing this)

[kdheepak]

If we are renaming these methods, can we do something like this instead?

pub trait RenderRef {
    type State;
    
    fn render_ref_with_state(&self,  area: Rect, buf: &mut Buffer, state: Option<&mut Self::State>) {
        // default empty implementation
    }

    fn render_ref(&self area: Rect, buf: &mut Buffer) {
        self.render_ref_with_state(area, buf, None);
    }
}

This way we can have just one trait, and get rid of the RenderRefWithState trait.

Widget developers would have to only implement RenderRef and can assume state is None if they want to implement the equivalent of the current Widget trait.

[kdheepak]

There's a tracking issue for associated type defaults which if merged would allow for a defaults for type State = (): rust-lang/rust#29661

pub trait RenderRef {
    type State = (); // `State` can default to empty tuple if tracking issue is merged
    
    fn render_ref_with_state(&self,  area: Rect, buf: &mut Buffer, state: Option<&mut Self::State>) {
        // default empty implementation
    }

    fn render_ref(&self area: Rect, buf: &mut Buffer) {
        self.render_ref_with_state(area, buf, None);
    }
}

And this will save one line for widgets that only want to implement render_ref.

[joshka]

On a full redo, I think I'd prefer something like:

trait Render {
    fn render(&self, context: &mut RenderContext);
}

trait RenderMut {
    fn render_mut(&mut self, context: &mut RenderContext);
}

But I'm not sure yet whether it's also worth including things like fn layout() and fn on_event() etc. Where's the right line to draw on this? (E.g. what's relevant from https://poignardazur.github.io/2023/02/02/masonry-01-and-my-vision-for-rust-ui/#the-widget-trait and linebender/masonry#26)

[EdJoPaTo]

Why not use a single trait and decide about ref or whatever on the implementation?
impl Whatever for &mut List

[kdheepak]

Can we combine them into one trait?

trait Render {
    fn render_mut(&mut self, context: &mut RenderContext) {
        self.render(context)
    }
    
    fn render(&self, context: &mut RenderContext) {
    }
}

Why not use a single trait and decide about ref or whatever on the implementation?

@EdJoPaTo I believe it was not possible to use Box<dyn Widget> if we let users decide ref on implementation. See this discussion by @joshka for more information about the problem: https://users.rust-lang.org/t/calling-a-method-with-a-self-parameter-on-box-dyn-trait/106220/5

For example, my understanding is that this code is not possible with the current Widget trait and needs render to take &self by reference. This PR has more information: #903

[thscharler]

Is StatefulWidgetRef useful for anything at all?

With the associated type State I can't have a

Vec<Box<dyn StatefulWidget>>

instead I would need a

Vec<Box<dyn StatefulWidget<State=WidgetState>>>.

Which means I can't have a Vec with mixed widgets, so why not just use

Vec<ConcreteWidget>

instead.

[thscharler]

Another thing is unclear to me.

If I create a composite widget I end up cloning the contained widget like in

use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use ratatui::style::{Style, Styled, Stylize};
use ratatui::text::Text;
use ratatui::widgets::StatefulWidgetRef;

#[derive(Default, Clone)]
struct Button<'a> {
    text: Text<'a>,
    style: Style,
}

struct ButtonState {
    pub armed: bool,
}

impl<'a> StatefulWidgetRef for Button<'a> {
    type State = ButtonState;

    fn render_ref(&self, area: Rect, buf: &mut Buffer, state: &mut Self::State) {
        // ...
    }
}

#[derive(Default, Clone)]
struct FocusableButton<'a> {
    widget: Button<'a>,
}

struct FocusableButtonState {
    pub widget: ButtonState,
    pub focus: bool,
}

impl<'a> StatefulWidgetRef for FocusableButton<'a> {
    type State = FocusableButtonState;

    fn render_ref(&self, area: Rect, buf: &mut Buffer, state: &mut Self::State) {
        let w = if state.focus {
            let mut w = self.widget.clone();
            w.style = w.style.on_light_blue();
            w
        } else {
            self.widget.clone()
        };
        w.render_ref(area, buf, &mut state.widget)
    }
}

In this case that clone() doesn't matter much, but if I have a List or Table instead this ends up cloning the whole thing.

Is this intended, or how am I supposed to do something like that?

[joshka]

Which means I can't have a Vec with mixed widgets

This is one of the reasons why this is unstable for now. I just made an example in tui-scrollview, where there are different widgets but they share the same state type. But I can see how this might be useless if they had to be different.

The context of this is:

• tui-rs had Widget, which consumes self
• tui-rs had StatefulWidget, which adds a &mut state variable
• We discovered that you could sort of work around the need for StatefulWidget by implementing Widget on &SomeType, and &mut SomeType, but haven't fully investigated the boundaries of what that enables
• We noticed that we needed a new type to enable Widget for boxed widgets. (this is still unstable)
• We added StatefulWidgetRef for consistency with WidgetRef. (also unstable)

[joshka]

If I create a composite widget I end up cloning the contained widget...

Tui-rs was created around the idea of recreating widgets on every frame, WidgetRef relaxes that idea somewhat, but it's not the only approach that works. Another approach is to have the state have a method that returns the widget, and ensure that the widget is fairly lightweight (i.e. the data is stored in the state, not the widget):

struct ButtonState {
  armed: bool,
}

struct ButtonWidget<'a> {
  style: Style,
  state: &'a ButtonState,
}

impl ButtonState {
  fn widget(&self) -> ButtonWidget {
    style: ...,
    state: self,
  }
}

impl FocusedButtonState {
  fn widget(&self) -> ButtonWidget {
    let button = self.button_state.widget();
    if ... { button.style = ... }
    button.render(...);
  }
}

Combine this with mutable refs where necessary.

[thscharler]

Which means I can't have a Vec with mixed widgets

This is one of the reasons why this is unstable for now. I just made an example in tui-scrollview, where there are different widgets but they share the same state type. But I can see how this might be useless if they had to be different.

The context of this is:

* tui-rs had Widget, which consumes self

* tui-rs had StatefulWidget, which adds a &mut state variable

* We discovered that you could sort of work around the need for StatefulWidget by implementing Widget on &SomeType, and &mut SomeType, but haven't fully investigated the boundaries of what that enables

* We noticed that we needed a new type to enable Widget for boxed widgets. (this is still unstable)

* We added StatefulWidgetRef for consistency with WidgetRef. (also unstable)

I didn't know the exact history of this all, good to know.

I tried, and found one way that might work:
If you make the state dyn Any as

pub trait AnyWidget: StatefulWidgetRef<State = dyn AnyWidgetState> {}
pub trait AnyWidgetState: Any {
    fn as_any_mut(&mut self) -> &mut dyn Any;
}

it almost works with StatefulWidgetRef, except this requires type State: ?Sized. Which wouldn't be a problem with StatefulWidgetRef as it is defined now.

And you need as_any_mut() to work around the unstable trait object upcasting.

I think I will stay with keep your widgets cheap for now.

[joshka]

I think I will stay with keep your widgets cheap for now.

At some point this might make a good writeup for the website. Especially combined with some of the ideas / code from https://forum.ratatui.rs/t/question-how-can-i-representing-applicatoin-state-ergonomically-in-ratatui-programs/54/4

[thscharler]

Which means I can't have a Vec with mixed widgets

This is one of the reasons why this is unstable for now. I just made an example in tui-scrollview, where there are different widgets but they share the same state type. But I can see how this might be useless if they had to be different.
The context of this is:

* tui-rs had Widget, which consumes self

* tui-rs had StatefulWidget, which adds a &mut state variable

* We discovered that you could sort of work around the need for StatefulWidget by implementing Widget on &SomeType, and &mut SomeType, but haven't fully investigated the boundaries of what that enables

* We noticed that we needed a new type to enable Widget for boxed widgets. (this is still unstable)

* We added StatefulWidgetRef for consistency with WidgetRef. (also unstable)

I didn't know the exact history of this all, good to know.

I tried, and found one way that might work: If you make the state dyn Any as

pub trait AnyWidget: StatefulWidgetRef<State = dyn AnyWidgetState> {}
pub trait AnyWidgetState: Any {
    fn as_any_mut(&mut self) -> &mut dyn Any;
}

it almost works with StatefulWidgetRef, except this requires type State: ?Sized. Which wouldn't be a problem with StatefulWidgetRef as it is defined now.

And you need as_any_mut() to work around the unstable trait object upcasting.

I think I will stay with keep your widgets cheap for now.

I've pondered a bit more about this dyn Any stuff. Once you add one or more traits to the state things start to get complicated really fast, without providing a satisfying solution.

Maybe this might work if we had one all encompassing trait for the widget, with all the batteries included. But as I tried working with two traits I came to the limits of the poor excuse for RTTI that we have with Any.

After my brain started fogging and my eyes drifted of into the 4th dimension, I remembered the first rule of rust: Try something else.

[joshka]

After my brain started fogging and my eyes drifted of into the 4th dimension, I remembered the first rule of rust: Try something else.

RIIR -> Rewrite it again when the borrow checker slaps you in the face

[EdJoPaTo]

Given the confusions about the render traits… Should it be a trait in the first place?

A struct feels long-term, so people expect them to be long-term.
Splitting this up might resolve some confusion here:

struct TableState {offset: usize, …}

// Should own its content, so no references needed. Probably created once for the whole time of the application.
struct TableOptions {highlight_style: Style, headers: Vec<Cell>, …}

fn ratatui::table::render(
  buffer: Buffer,
  area: Rect,
  state: &mut TableState,
  rows: &[Row],
  &options: &TableOptions,
) {…}

The downside is the grouping of many widgets in a Vec like @thscharler tried to achieve.

Having a struct named clearly for this purpose with a single trait for this would achieve that too:

// Keep renferences on everything instead of owning things
struct TableRenderInformation<'a> {
  state: &'a mut TableState,
  rows: &'a [Row],
  options: &'a TableOptions,
}

impl Render for TableRenderInformation {
  // self as the TableRenderInformation should only keep references.
  fn render(self, area: Rect, buffer: Buffer);
}

This would also remove the need for the Context suggested in #1044 as every widget knows which information like state or not are required to render.
Rendering widgets without a specific state can still use state: State::default() there.

[thscharler]

Given the confusions about the render traits… Should it be a trait in the first place?

A struct feels long-term, so people expect them to be long-term. Splitting this up might resolve some confusion here:

struct TableState {offset: usize, …}

// Should own its content, so no references needed. Probably created once for the whole time of the application.
struct TableOptions {highlight_style: Style, headers: Vec<Cell>, …}

fn ratatui::table::render(
  buffer: Buffer,
  area: Rect,
  state: &mut TableState,
  rows: &[Row],
  &options: &TableOptions,
) {…}

The downside is the grouping of many widgets in a Vec like @thscharler tried to achieve.

Not only that, but you'll need a trait for widgets like a ScrollPanel or a Viewport too.
Any case where you have a

Container<T> {
    inner: T
}

you'll need a trait to render the inner T.

Having a struct named clearly for this purpose with a single trait for this would achieve that too:

// Keep renferences on everything instead of owning things
struct TableRenderInformation<'a> {
  state: &'a mut TableState,
  rows: &'a [Row],
  options: &'a TableOptions,
}

impl Render for TableRenderInformation {
  // self as the TableRenderInformation should only keep references.
  fn render(self, area: Rect, buffer: Buffer);
}

This would also remove the need for the Context suggested in #1044 as every widget knows which information like state or not are required to render. Rendering widgets without a specific state can still use state: State::default() there.

This wouldn't remove the need for Context, but the need for StatefulWidget.
On a glance this might work to remove the need for StatefulWidget and make everything a Widget.
Or you could go the other way round and say that a Widget is just a StatefulWidget<State=()>.

The point for the Context was more to have extra information like the frame-count or the cursor position.
Or whatever else will come up.

On a sidenote, I wonder why it was never considered to use Frame
instead of Buffer as argument to render(). This would at least solve the current problems with frame-count
and setting the cursor, and could probably extended for any extra needs.

[joshka]

Splitting this up might resolve some confusion here:

struct TableState {offset: usize, …}

// Should own its content, so no references needed. Probably created once for the whole time of the application.
struct TableOptions {highlight_style: Style, headers: Vec<Cell>, …}

fn ratatui::table::render(
  buffer: Buffer,
  area: Rect,
  state: &mut TableState,
  rows: &[Row],
  &options: &TableOptions,
) {…}

This is effectively isomorphic (or functionally equivalent) to what we already have.

• TableOptions is named Table (options is a meaningless word in this context)
• table::render is named StatefulWidgetRef::render_ref (options is self), but you've moved the rows parameter out of the table options into the method params.

Having a struct named clearly for this purpose with a single trait for this would achieve that too:

// Keep renferences on everything instead of owning things
struct TableRenderInformation<'a> {
  state: &'a mut TableState,
  rows: &'a [Row],
  options: &'a TableOptions,
}

impl Render for TableRenderInformation {
  // self as the TableRenderInformation should only keep references.
  fn render(self, area: Rect, buffer: Buffer);
}

I've used a similar approach before (I can't find the code right now - it's likely I migrated that code from something like struct AppWidget { app: &App } to impl Widget for &App ). It's a useful approach, but unnecessary to complicate things if your render method takes a mutable reference instead of consuming self.

Table {
   state: TableState,
   rows: Vec<Row>,
   options: Options
}

impl WidgetMut for Table {
  fn render_mut(&mut self, area: Rect, buffer: Buffer);
}

This would also remove the need for the Context suggested in #1044 as every widget knows which information like state or not are required to render. Rendering widgets without a specific state can still use state: State::default() there.

Context is the context of the rendering process, not the context for the widget. It should contain things like what frame we're on, when was the last frame rendered, the buffer, precomputed layout information, ...

On a sidenote, I wonder why it was never considered to use Frame instead of Buffer as argument to render().

Basically historical reasons. If I'd redesign Ratatui entirely based on current knowledge:

• Widgets would be implement multiple methods for layout, handling events, rendering, etc.
• Widget methods would have access to a context that helps them play nicely with each other
• Buffer would be a trait, to make it possible to give portions of a frame to write to instead of the entire frame
• A bunch of other things.

But we've got what we've got, and so we implement things in an additive way that hopefully avoids breaking the world.

[EdJoPaTo]

This is effectively isomorphic (or functionally equivalent) to what we already have.

Thats the point.

A struct feels long-term, so people expect them to be long-term.
Splitting this up might resolve some confusion here