feat(text): support conversion from Display to Span, Line and Text

[orhun]

Now you can create Line and Text from numbers like so:

let line = 42.to_line();
let text = 666.to_text();

(I was doing little testing for my TUI app and saw that this isn't supported - then I was like WHA and decided to make it happen ™️)

[orhun]

I also looked into using something more generic (like Num trait from num-traits) but felt like an overkill at the end.

[joshka]

This doesn't seem like something that would be expected idiomatically in rust.

• String::from(num) doesn't exist
• Cow<str>::from(num) also doesn't

I'd expect to generally use format! for this rather than adding conversions from random things to strings. (And the ratatui-macros have some macros that call into format - line!, text!, span! if you want something shorter)

Perhaps this could be implemented as traits: ToSpan / ToLine / ToText if we're going for something more aligned with existing rust idioms, and then those would be implemented on num etc.

[EdJoPaTo]

What about creating a Span from every Display implementation?

[joshka]

What about creating a Span from every Display implementation?

I like the idea - I'd be concerned about whether this is too magical for newer users. Any ideas about whether there could be other downsides of this?

[orhun]

This doesn't seem like something that would be expected idiomatically in rust.

Yeah, I see your point & makes sense. I'm happy to take the ToLine approach here.

What about creating a Span from every Display implementation?

Can you elaborate this? I didn't quite get what this would look like.

[joshka]

Can you elaborate this? I didn't quite get what this would look like.

perhaps either:

impl<T:Display> From<T> for Span { ... }
// or
impl <T: Display> ToSpan for T { ... }

I suspect the first approach might clash with other existing / future generic implementations, so it's something to be careful with. It's a reasonable idea that's worth exploring, but I'd want to see how it pans out in real use cases to evaluate whether it really works well.

[EdJoPaTo]

Maybe a more explicit Span::from_display<T: Display>(input: T) might be helpful for readable and understandable code.

[joshka]

Maybe a more explicit Span::from_display<T: Display>(input: T) might be helpful for readable and understandable code.

The ToSpan trait idea is more in line with ToString in std.

https://doc.rust-lang.org/std/string/struct.String.html#impl-ToString-for-T

impl<T: fmt::Display + ?Sized> ToString for T { ... }

So (copying wording etc. from std too):

/// A trait for converting a value to a [`Span`].
///
/// This trait is automatically implemented for any type that implements the [`Display`] trait. As
/// such, `ToSpan` shouln't be implemented directly: [`Display`] should be implemented instead, and
/// you get the `ToSpan` implementation for free.
///
/// [`Display`]: std::fmt::Display
pub trait ToSpan {
    fn to_span(&self) -> Span<'_>;
}

/// # Panics
///
/// In this implementation, the `to_span` method panics if the `Display` implementation returns an
/// error. This indicates an incorrect `Display` implementation since `fmt::Write for String` never
/// returns an error itself.
impl<T: fmt::Display> ToSpan for T {
    fn to_span(&self) -> Span<'_> {
        Span::raw(self.to_string())
    }
}

#[test]
fn test_num() {
    assert_eq!(42.to_span(), Span::raw("42"));
}

... and ToLine / ToText follow pretty nicely.

[orhun]

That looks good! Implemented in bdb2096

However, this is not exactly what I wanted to see:

-let text = Text::from(42);
+let text = Text::from(42.to_span());

To take the value directly I'm guessing we need impl<T: Display> for Text but that conflicts with the other implementations.

[joshka]

However, this is not exactly what I wanted to see:

-let text = Text::from(42);
+let text = Text::from(42.to_span());

What about adding ToLine and ToText?

let text = 42.to_text();

[kdheepak]

I like the .to_span() idea but I don't think we should do .to_line() and .to_text() because it is non intuitive that

• 42.to_line() becomes [Span::new(42.to_string())] and
• 42.to_text() becomes [ [Span::new(42.to_string())] ]

I'd prefer upstreaming the macros from ratatui-macros after adding a .to_span() method. Then users can write:

span!(42)
// or
line![42]
// or
text![42]

[joshka]

I like the .to_span() idea but I don't think we should do .to_line() and .to_text() because it is non intuitive that

* `42.to_line()` becomes `[Span::new(42.to_string())]` and

* `42.to_text()` becomes `[ [Span::new(42.to_string())] ]`

They would create a Line and a Text, not arrays. I think these extra traits seem to add some consistency. In particular, if you know that the Display implementation of something is multi-line, then you should expect to be able to convert that to text, not to a span / line.

I'd prefer upstreaming the macros from ratatui-macros after adding a .to_span() method. Then users can write:

span!(42)
// or
line![42]
// or
text![42]

This is not necessarily an alternative - wouldn't it make sense to do both?

[EdJoPaTo]

to_span and to_text seem reasonable to me. to_line seems pointless, as a Line consists of multiple Spans with different styling. But there can only be a single style assigned here (the raw / unset one).

The only difference of to_text and to_span would be the handling of new lines.

[joshka]

to_line seems pointless, as a Line consists of multiple Spans with different styling.

As a direct conversion perhaps not, but as an intermediate value where a line is composed, or just as a more obviously explicit call where a line is expected

let mut line = 42.to_line();
line.push_span("%".red());

let mut text = Text::from("asdf");
text.push_line(42.to_line());

I definitely agree that ToSpan and ToText have strong reasons to belong. ToLine is a weaker reason, but still useful (and for consistency this makes sense). There's no obvious downside to this.

Handling newlines should be the same as for Line - they get swallowed afaik (noting that there might be some ways to get newline characters in there).

If there is no to_line then the latter might be written as:

text.push_line(42.to_span());
// which does something completely different (but not obviously so) to:
text.push_span(42.to_span());

[EdJoPaTo]

let mut line = 42.to_line();
line.push_span("%".red());

When the length is already known it's also more performant to write it differently:

let line = Line::from([42.to_span(), "%".red()];

to_line would only be a convenience thing, but a single span should always work as a line, so to_line would be pointless.

---

If there is no to_line then the latter might be written as: […]

As long as the concept of Span, Line, and Text are clear, this isn't confusing. push_line accepts Into<Line> so push_line(42.to_span()) should work as expected.

When the concept isn't clear, Into is introducing confusing behavior. So these three thingies should be explained well enough, then I don't see an issue here.

[joshka]

let mut line = 42.to_line();
line.push_span("%".red());

When the length is already known it's also more performant to write it differently:

let line = Line::from([42.to_span(), "%".red()];

to_line would only be a convenience thing, but a single span should always work as a line, so to_line would be pointless.

If there is no to_line then the latter might be written as: […]

As long as the concept of Span, Line, and Text are clear, this isn't confusing. push_line accepts Into<Line> so push_line(42.to_span()) should work as expected.

When the concept isn't clear, Into is introducing confusing behavior. So these three thingies should be explained well enough, then I don't see an issue here.

Ed, you're straw-manning the argument. The examples are intentionally simplified. Try strong-manning instead and see if you're able to find some examples that do make sense for ToLine to exist. Particularly try taking on the new rust user perspective.

[kdheepak]

They would create a Line and a Text, not arrays.

My mental model is that we have Span, const Line = Vec<Span>, const Text = Vec<Line>.

In particular, if you know that the Display implementation of something is multi-line, then you should expect to be able to convert that to text, not to a span / line.

Having something that can convert multiple lines separated by \n in the display output has me convinced of .to_text(). I'm indifferent towards .to_line(). Maybe just for the sake on consistency it should exist?

[matta]

One way to approach deciding these kinds of API questions is to go find larger examples, in code that is already out there today, that would be improved if it could use the new API. It is difficult to assess whether small examples are good ideas in isolation, and easier if, say, a 10-20 line code sample is improved in some way.

[joshka]

Thinking about this more, I think there's a bit of a problem with using the Display for any of this - what if you'd want to put styling info directly on the type you're dealing with instead of external to the type. Let's take an example like:

struct MarkdownHeading {
    content: String,
    ...
}

impl fmt::Display for MarkdownHeading {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(f, "{}", content)
    }
}

// but what if I want this to be always styled

impl ToLine for MarkdownHeading {
    fn to_line(&self) -> Line {
        content.blue().bold().into()
    }
}

The same could be said for Span and Text easily - if we implement ToXXX for Display, then we prevent the ability for types to implement their own conversion to Span/Line/Text through these methods. Perhaps these traits might be better named ToRawSpan, ToRawLine, ToRawText?

My mental model is that we have Span, const Line = Vec<Span>, const Text = Vec<Line>.

Ah, I see. Mine is that these are all just Strings, but with formatting and alignment. The internal representation should be treated as an implementation detail.

Where I really do like this idea is that it provides a more natural way to compose text/line oriented things (like the markdown example, or writing pretty printed JSON). If you have a container of things which implement trait MyCustomThing: ToLine, then you can iterate over that for rendering nicely. In https://github.com/joshka/ratatui-json-tutorial/ I was trying to work out ways to convert the json tree into a tree of widgets, but got stuck thinking about how to keep track of where each element widget starts and finishes on a line. In this situation, switching to treating them as being convertible to individual lines / spans seems a logical step.

[matta]

Thinking about this more, I think there's a bit of a problem with using the Display for any of this - what if you'd want to put styling info directly on the type you're dealing with instead of external to the type.

It is probably better to talk about doing this for the std::string::ToString trait instead of std::fmt::Display trait, since at least in that case it is clear there are zero available formatting options.

But I agree with you about the bit of a problem.

I usually think it is a good practice to define a trait/protocol/interface next to its consumer. That is the case with std::fmt::Display: it is defined in std::fmt for the purpose of std::fmt::format!.

The benefit of this approach is that the two can be designed together. For example, std::fmt::Display implementations are given a Formatter, which has all sorts of stuff related to formatting numbers, debug strings, etc., in support formatting options set on the format string. By omitting the format string, and just calling Display, flexibility is lost. At the same time, stuff related to rendering into terminals is absent (colors, underline, blink, cursor position, etc.).

It might be convenient to take advantage of a type's std::fmt::Display trait for rendering in Ratatui (or other UIs), but that is perhaps done in an opt-in way. A pretty clear, obvious way is to require a call to format!, which has the advantage of allowing the above-mentioned std::fmt options to be set on the Formatter passed to Display::fmt.

...and if talking about supporting the ToString trait, how about just having application code call thing.to_string() instead?

Where I really do like this idea is that it provides a more natural way to compose text/line oriented things (like the markdown example, or writing pretty printed JSON). If you have a container of things which implement trait MyCustomThing: ToLine, then you can iterate over that for rendering nicely. In https://github.com/joshka/ratatui-json-tutorial/ I was trying to work out ways to convert the json tree into a tree of widgets, but got stuck thinking about how to keep track of where each element widget starts and finishes on a line. In this situation, switching to treating them as being convertible to individual lines / spans seems a logical step.

Analogous to std::fmt that would be a trait under the ratatui:: module for Ratatui's purposes. Makes a lot of sense to me.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.4%. Comparing base (38bb196) to head (67718c5).

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #1167   +/-   ##
=====================================
  Coverage   94.4%   94.4%           
=====================================
  Files         62      62           
  Lines      14941   14959   +18     
=====================================
+ Hits       14110   14128   +18     
  Misses       831     831           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[orhun]

I personally like the idea of having ToXyz implementations for Line, Span and Text. I implemented that in my latest commit along with some tests.

I'm happy to rename it to ToRawXyz, but I think having this in the first place is a good start.

what if you'd want to put styling info directly on the type you're dealing with instead of external to the type.

Can't we export the trait and let the users implement it for their own types? Also, we should think about how common that use-case is too.

[matta]

what if you'd want to put styling info directly on the type you're dealing with instead of external to the type.

Can't we export the trait and let the users implement it for their own types? Also, we should think about how common that use-case is too.

See https://github.com/rust-lang/rfcs/blob/master/text/0565-show-string-guidelines.md#user-facing-fmtdisplay, which talks about how it is expected that application code will use a different type (most likely a newtype) for each variation of Display behavior they want.

[joshka]

what if you'd want to put styling info directly on the type you're dealing with instead of external to the type.

Can't we export the trait and let the users implement it for their own types? Also, we should think about how common that use-case is too.

See https://github.com/rust-lang/rfcs/blob/master/text/0565-show-string-guidelines.md#user-facing-fmtdisplay, which talks about how it is expected that application code will use a different type (most likely a newtype) for each variation of Display behavior they want.

I think what you're saying is if the widget implements display, and that's not what we want to be rendered, then it's easy enough to do MyWidget::for_whatever().to_text() etc., so we shouldn't worry too much about the blanket Display implementation being a blocker.

@orhun we can export the trait, but my point was that you can't implement it twice on the one type, and implementing it as a blanket implementation for every type that implements display would be the first implementation in some cases. I'd guess this is probably rare enough that it shouldn't matter too much though (and the workaround above is fine).

[orhun]

Addressed the review comments and rebased on main. Now the only issue is:

warning: method `to_line` is never used
warning: method `to_span` is never used
warning: method `to_text` is never used

Should we come up places to use this new mechanism somewhere in the code or just slap dead_code on it? What do you think @ratatui-org/maintainers

[matta]

Now the only issue is:

warning: method `to_line` is never used
warning: method `to_span` is never used
warning: method `to_text` is never used

Should we come up places to use this new mechanism somewhere in the code or just slap dead_code on it? What do you think @ratatui-org/maintainers

Nothing pub should warn about being unused. A pub thing counts as being (possibly) used by the users of the crate.

It took me a while to figure this out, but it turns out that the new traits were not actually pub outside Ratatui. Try the patch below (which causes new warnings about no doc comment for the trait methods):

diff --git a/src/text.rs b/src/text.rs
index 6262ca6..92aad3a 100644
--- a/src/text.rs
+++ b/src/text.rs
@@ -49,13 +49,16 @@ pub use grapheme::StyledGrapheme;

 mod line;
 pub use line::Line;
+pub use line::ToLine;

 mod masked;
 pub use masked::Masked;

 mod span;
 pub use span::Span;
+pub use span::ToSpan;

 #[allow(clippy::module_inception)]
 mod text;
 pub use text::Text;
+pub use text::ToText;

[joshka]

Check clippy

[orhun]

Thanks for the pointers guys, it should be good to go now!