feat: add WidgetExt trait for debugging widgets #1065
Conversation
joshka
requested review from
orhun,
mindoodoo,
sayanarijit,
kdheepak,
Valentin271 and
EdJoPaTo
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1065 +/- ##
=======================================
- Coverage 89.4% 89.2% -0.3%
=======================================
Files 61 63 +2
Lines 15464 15615 +151
=======================================
+ Hits 13833 13936 +103
- Misses 1631 1679 +48 ☔ View full report in Codecov by Sentry. |
Importing the `WidgetExt` trait allows users to easily get a string
representation of a widget with ANSI escape sequences for the terminal.
This is useful for debugging and testing widgets.
```rust
use ratatui::{prelude::*, widgets::widget_ext::WidgetExt};
fn main() {
let greeting = Text::from(vec![
Line::styled("Hello", Color::Blue),
Line::styled("World ", Color::Green),
]);
println!("{}", greeting.to_ansi_string(5, 2));
}
```
Fixes: #1045
|
Made AnsiStringBuffer less private so this works too: use ratatui::{prelude::*, widgets::ansi_string_buffer::AnsiStringBuffer};
fn main() {
let mut buf = AnsiStringBuffer::new(5, 2);
buf.render_ref(&Line::styled("Hello", Color::Blue), Rect::new(0, 0, 5, 1));
buf.render_ref(&Line::styled("World", Color::Green), Rect::new(0, 1, 5, 1));
println!("{buf}");
} |
|
Things I'm not 100% sure of with this (and the rationale for this being "unstable"):
|
| feature = "widget-ext", | ||
| issue = "https://github.com/ratatui-org/ratatui/issues/1045" | ||
| )] | ||
| pub trait WidgetExt { |
There was a problem hiding this comment.
I agree with your comment about the naming. In the off chance we want to add more extensions in the future, this should be named ToAnsiStringWidgetExt or something to that effect.
There was a problem hiding this comment.
Either this trait should allow for more things on the same trait (in the future), then the name is fine, or it should be specific to this.
I also dislike the Ext ending here.
AnsiStringPrintable would be a trait name that represents what it does.
There was a problem hiding this comment.
I think Ext is probably the right suffix for this pattern if it's not a more generic trait. https://rust-lang.github.io/rfcs/0445-extension-trait-conventions.html. It's used in tokio / futures streams (StreamExt), tracing-subscriber (a bunch of times https://docs.rs/tracing-subscriber/latest/tracing_subscriber/?search=ext) and various other crates use similar naming. I mostly wasn't sure if adding something before Ext made sense.
The proposed convention is, first of all, to (1) prefer adding default methods to existing traits or (2) prefer generically useful traits to extension traits whenever feasible.
Adding default methods doesn't seem like the right idea as converting to ANSI doesn't seem like it's something everyone would need on a widget, and it's something I'd prefer not to confuse newer users with if possible (hence adding as an extension trait / generically useful trait seems right). Adding a default method would however allow each widget to define their own ANSI representation (though I can't think of a useful need for this).
I'm not sure that given that the method needs a certain size to render, whether this trait has a generic usefulness. I don't see that there's really any need for this trait other than printing out widgets (I could be wrong there).
There was a problem hiding this comment.
Other objects not being Widgets could implement that trait too. Buffer for example.
There was a problem hiding this comment.
I was also thinking Ext was the right suffix since I had seen it in several other places, but maybe it is more commonly used when extending external traits or standard library traits? I like AnsiStringPrintable too.
If it has just one method .to_ansi_string(), why not trait ToAnsiString {}?
There was a problem hiding this comment.
Other objects not being Widgets could implement that trait too. Buffer for example.
The bit I struggle with is working out what the purpose of passing in the width and height would be if implemented on Buffer? Would trait RenderToAnsiString { fn render_to_ansi_string() } make sense?
| @@ -21,6 +21,7 @@ | |||
| //! - [`Tabs`]: displays a tab bar and allows selection. | |||
| //! | |||
| //! [`Canvas`]: crate::widgets::canvas::Canvas | |||
| pub mod ansi_string_buffer; | |||
There was a problem hiding this comment.
I want to mark this as unstable and the widget module and re-export the trait / struct, but this needs sagebind/stability#11 (enable unstable attribute on use statements).
Importing the
WidgetExttrait allows users to easily get a stringrepresentation of a widget with ANSI escape sequences for the terminal.
This is useful for debugging and testing widgets.
Fixes: #1045