Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(paragraph): allow Lines to be individually aligned #149

Merged
merged 4 commits into from
May 26, 2023

Conversation

TimerErTim
Copy link
Contributor

@TimerErTim TimerErTim commented Apr 25, 2023

For a small project of mine I needed to support different Alignments (Center, Left and Right) not only on whole paragraphs but also on Lines within these paragraphs.

So I implemented that in my local copy of the project and thought, why not contribute that effort to the original project. So here is my PR:

Content

I added a new field to the Line struct containing information about the Alignment. By default, if no alignment is specified, the one from the rendering Paragraph will be used.
I had to rewrite the LineComposers to operate on a line-by-line level and not symbol-by-symbol, since the alignment information is bound to the line the Line struct represents.

Furthermore, the Line's spans have been made private so future additions will not cause a breaking change. Access methods were written as a substitution.

Testing

This PR also adds many new tests that help to ensure correct paragraph and line composing behavior.

These tests were added and successfully run before the implementation of the new alignment feature. After the new implementations the tests were run again without alteration. This should ensure behavioral stability, but cannot guarantee it, as tests inherently cannot prove the absence of bugs but merely their presence.

Metastuff

I did not do any performance checks or benchmarks, neither pre nor post feature implementation. Time complexity should be around O(n) for symbol count.

The new algorithms are documented in source code with comments, and if anything is unclear or needs further documentation, I am eager to help.

Implication

BREAKING CHANGE: The hiding of a previously public field in the Line struct breaks API backwards compatibility.

@TimerErTim TimerErTim force-pushed the spans-alignment branch 4 times, most recently from 9f73adb to e420092 Compare April 25, 2023 15:23
src/text.rs Outdated Show resolved Hide resolved
src/widgets/paragraph.rs Outdated Show resolved Hide resolved
src/widgets/reflow.rs Outdated Show resolved Hide resolved
tests/widgets_paragraph.rs Show resolved Hide resolved
tests/widgets_paragraph.rs Outdated Show resolved Hide resolved
@TimerErTim TimerErTim changed the title Add alignments to Spans feat(paragraph)!: add alignments to Spans Apr 29, 2023
@TimerErTim
Copy link
Contributor Author

@joshka
I am currently rewriting the unit tests inside the paragraph.rs file. Should I amend to your commit and merge all tests with this (#149 ) PR or simply add another commit that extends upon yours?

Copy link
Collaborator

@sophacles sophacles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat feature idea.

Does this cause any compatability concerns since it changes the Span? (i.e. is this a breaking change)?

src/text.rs Outdated Show resolved Hide resolved
@TimerErTim
Copy link
Contributor Author

Yes, it is a breaking change, unfortunately.
It's impact on consumers should be minimal though, since one would generally use the Spans::from method anyways.

@joshka joshka self-requested a review May 4, 2023 12:03
@sophacles
Copy link
Collaborator

It's impact on consumers should be minimal though

I agree, Im not opposed to breaking changes, but they should be noted in commit logs - if you do a rebase or squash please add a note :)

@TimerErTim
Copy link
Contributor Author

It's impact on consumers should be minimal though

I agree, Im not opposed to breaking changes, but they should be noted in commit logs - if you do a rebase or squash please add a note :)

So you mean in addition to the breaking change commit header (annotated with the !) it should also be clarified in the footer?

@sophacles
Copy link
Collaborator

sophacles commented May 4, 2023

Yeah, something in the footer like

BREAKING CHANGE: Spans moved from a tuple struct to one with named fields

to explain the break is. It doesn't have to be extremely detailed, just enough to let someone looking through the changelog (or git log) knew where to look in thier code or rustc output when they build against the new version.

@joshka joshka added the enhancement New feature or request label May 5, 2023
Copy link
Member

@joshka joshka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only 100% necessary change I'd want to see is to name the configuration for this Spans::alignment() rather than aligned(). All my other comments are nit picks / questions.

I like this change a lot. I only read it in passing last week, and I think I didn't expand out the unit tests in paragraph as github showed it as collapsed. They tests are really well written, and easy to understand / maintain. Today I read everything but the reflow changes.

Taking a look at the coverage using cargo tarpaulin, reflow has 132/135 lines covered (98%) and paragraph has 61/65 (93%). That's huge!

Can you take a quick look at whether the uncovered lines are critical? In paragraph.rs, They are Paragraph.style() (which is probably simple to add?) https://github.com/tui-rs-revival/ratatui/blob/91f90fedbad4098426849e7813be5676b7a00c11/src/widgets/paragraph.rs#L114-L117
There is also https://github.com/tui-rs-revival/ratatui/blob/91f90fedbad4098426849e7813be5676b7a00c11/src/widgets/paragraph.rs#L181-L186

In reflow.rs the following line is uncovered https://github.com/tui-rs-revival/ratatui/blob/91f90fedbad4098426849e7813be5676b7a00c11/src/widgets/reflow.rs#L142-L144

I still don't really understand much of the reflow logic (it seems complex). But given that the test coverage is so high, my understanding doesn't seem to matter as the implementation still does the same things as it used to do with the addition of the new functionality.

Can you please indicate the impact of the breaking change in the conventional commit footer (I know it's small, but if it breaks someone it would be nice for them to be able to point to a commit that tells them how to fix their usage of the Spans type)

If you're rewriting commit messages anyway, there's a test conventional commit tag you can use. No big deal on that.

Thanks for the effort you put into this change. It looks great!

I'll slap an approval on this once the fix mentioned for aligned() is in and the breaking change info is added to the feat commit.

src/text.rs Outdated Show resolved Hide resolved
src/text.rs Outdated Show resolved Hide resolved
src/text.rs Outdated Show resolved Hide resolved
src/widgets/barchart.rs Outdated Show resolved Hide resolved
src/widgets/calendar.rs Outdated Show resolved Hide resolved
src/widgets/paragraph.rs Show resolved Hide resolved
src/widgets/paragraph.rs Show resolved Hide resolved
src/widgets/paragraph.rs Outdated Show resolved Hide resolved
src/widgets/reflow.rs Outdated Show resolved Hide resolved
src/widgets/reflow.rs Outdated Show resolved Hide resolved
@TimerErTim
Copy link
Contributor Author

I included the Paragraph::style method in the coverage. The others do not seem to be that critical, the last one actually is never reachable under normal circumstances, but I included it anyway in my rewrite just in case.

@TimerErTim TimerErTim force-pushed the spans-alignment branch 3 times, most recently from 609cc5c to 918fe20 Compare May 7, 2023 21:46
@TimerErTim TimerErTim requested a review from joshka May 8, 2023 10:46
Copy link
Member

@joshka joshka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for your effort in getting this one over the line @TimerErTim

@TimerErTim TimerErTim requested a review from joshka May 11, 2023 12:16
@TimerErTim
Copy link
Contributor Author

Sorry, didn't mean to request a re-review. Was just wondering why there are pending requested changes and misclicked.

@joshka
Copy link
Member

joshka commented May 22, 2023

+1 it's not a hassle to wait for the next release.

@TimerErTim
Copy link
Contributor Author

I would have worked on this today either way. I am not rushing, just informing you :)

The decision is yours, I don't mind. However bear in mind that we could introduce Alignment to Lines without a breaking change if this PR is merged before the next release. Otherwise the additional field on Line after its introduction release would again mean a breaking change.

@joshka
Copy link
Member

joshka commented May 22, 2023

I wonder if we maybe should implement Iterator / IntoIter / iter_mut on Line instead of making the spans public? This would have the advantage of allowing any fields to be added without a problem. Regardless though, adding a non public field shouldn't break (unless I'm mistaken).

@sayanarijit
Copy link
Member

adding a non public field shouldn't break

If users are initialising the struct directly, for whatever reason, it will break.

@TimerErTim
Copy link
Contributor Author

TimerErTim commented May 22, 2023

If we break initialization of Line after the next release, let's do it so future releases do not have to in order to add new features.

I'd make the spans private and follow @joshka 's suggestions, so users have to init Lines with a method.

Thoughts?

joshka and others added 3 commits May 22, 2023 22:32
Wrote a universal helper method to easily add new test cases.
Avoids boilerplate in new test methods.
Copy link
Member

@joshka joshka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with comments that are non-blocking. I think I was probably wrong about making spans() / spans_mut() methods, but I'm not 100% sure. If you concur, please drop that back to just making the spans field public and I'll approve the change as well.

src/text/spans.rs Show resolved Hide resolved
src/text/line.rs Outdated Show resolved Hide resolved
@joshka
Copy link
Member

joshka commented May 23, 2023

Here's the diff of rebasing the previously approved version of this PR on main (and just blindly accepting all conflicts) vs the current version of this PR. This is to make it easier to review just the difference rather than doing a full review.

diff --git a/src/buffer.rs b/src/buffer.rs
index 49ab756..0647eba 100644
--- a/src/buffer.rs
+++ b/src/buffer.rs
@@ -312,7 +312,7 @@ impl Buffer {
     pub fn set_spans(&mut self, x: u16, y: u16, spans: &Spans<'_>, width: u16) -> (u16, u16) {
         let mut remaining_width = width;
         let mut x = x;
-        for span in &spans.content {
+        for span in &spans.0 {
             if remaining_width == 0 {
                 break;
             }
@@ -333,7 +333,7 @@ impl Buffer {
     pub fn set_line(&mut self, x: u16, y: u16, line: &Line<'_>, width: u16) -> (u16, u16) {
         let mut remaining_width = width;
         let mut x = x;
-        for span in &line.spans {
+        for span in line.spans() {
             if remaining_width == 0 {
                 break;
             }
diff --git a/src/text.rs b/src/text.rs
index 86c973e..77d8ef5 100644
--- a/src/text.rs
+++ b/src/text.rs
@@ -46,13 +46,17 @@
 //!     Span::raw(" title"),
 //! ]);
 //! ```
-use std::borrow::Cow;
 
+use crate::style::Style;
+use std::{borrow::Cow, fmt::Debug};
 use unicode_segmentation::UnicodeSegmentation;
 use unicode_width::UnicodeWidthStr;
 
-use crate::layout::Alignment;
-use crate::style::Style;
+mod line;
+mod masked;
+mod spans;
+#[allow(deprecated)]
+pub use {line::Line, masked::Masked, spans::Spans};
 
 /// A grapheme associated to a style.
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -233,153 +237,6 @@ impl<'a> From<&'a str> for Span<'a> {
     }
 }
 
-/// A string composed of clusters of graphemes, each with their own style.
-#[derive(Debug, Clone, PartialEq, Default, Eq)]
-pub struct Spans<'a> {
-    pub content: Vec<Span<'a>>,
-    pub alignment: Option<Alignment>,
-}
-
-impl<'a> Spans<'a> {
-    /// Returns the width of the underlying string.
-    ///
-    /// ## Examples
-    ///
-    /// ```rust
-    /// # use ratatui::text::{Span, Spans};
-    /// # use ratatui::style::{Color, Style};
-    /// let spans = Spans::from(vec![
-    ///     Span::styled("My", Style::default().fg(Color::Yellow)),
-    ///     Span::raw(" text"),
-    /// ]);
-    /// assert_eq!(7, spans.width());
-    /// ```
-    pub fn width(&self) -> usize {
-        self.content.iter().map(Span::width).sum()
-    }
-
-    /// Changes the [`Alignment`] of this string.
-    /// The alignment determines on which side to render this line inside a
-    /// [`Paragraph`](crate::widgets::Paragraph). It overrides the default alignment that would
-    /// normally be assigned by the [`Paragraph`](crate::widgets::Paragraph).
-    ///
-    ///
-    /// ## Examples
-    ///
-    /// ```rust
-    /// # use ratatui::layout::Alignment;
-    /// # use ratatui::text::{Span, Spans};
-    /// # use ratatui::style::{Color, Style};
-    /// let spans = Spans::from(vec![
-    ///     Span::styled("My", Style::default().fg(Color::Yellow)),
-    ///     Span::raw(" text"),
-    /// ]).alignment(Alignment::Right);
-    /// assert_eq!(Some(Alignment::Right), spans.alignment);
-    /// ```
-    pub fn alignment(self, alignment: Alignment) -> Self {
-        Self {
-            alignment: Some(alignment),
-            ..self
-        }
-    }
-
-    /// Patches the style of each Span in an existing Spans, adding modifiers from the given style.
-    ///
-    /// ## Examples
-    ///
-    /// ```rust
-    /// # use ratatui::text::{Span, Spans};
-    /// # use ratatui::style::{Color, Style, Modifier};
-    /// let style = Style::default().fg(Color::Yellow).add_modifier(Modifier::ITALIC);
-    /// let mut raw_spans = Spans::from(vec![
-    ///     Span::raw("My"),
-    ///     Span::raw(" text"),
-    /// ]);
-    /// let mut styled_spans = Spans::from(vec![
-    ///     Span::styled("My", style),
-    ///     Span::styled(" text", style),
-    /// ]);
-    ///
-    /// assert_ne!(raw_spans, styled_spans);
-    ///
-    /// raw_spans.patch_style(style);
-    /// assert_eq!(raw_spans, styled_spans);
-    /// ```
-    pub fn patch_style(&mut self, style: Style) {
-        for span in &mut self.content {
-            span.patch_style(style);
-        }
-    }
-
-    /// Resets the style of each Span in the Spans.
-    /// Equivalent to calling `patch_style(Style::reset())`.
-    ///
-    /// ## Examples
-    ///
-    /// ```rust
-    /// # use ratatui::text::{Span, Spans};
-    /// # use ratatui::style::{Color, Style, Modifier};
-    /// let mut spans = Spans::from(vec![
-    ///     Span::styled("My", Style::default().fg(Color::Yellow)),
-    ///     Span::styled(" text", Style::default().add_modifier(Modifier::BOLD)),
-    /// ]);
-    ///
-    /// spans.reset_style();
-    /// assert_eq!(Style::reset(), spans.content[0].style);
-    /// assert_eq!(Style::reset(), spans.content[1].style);
-    /// ```
-    pub fn reset_style(&mut self) {
-        for span in &mut self.content {
-            span.reset_style();
-        }
-    }
-}
-
-impl<'a> From<String> for Spans<'a> {
-    fn from(s: String) -> Spans<'a> {
-        Spans {
-            content: vec![Span::from(s)],
-            ..Spans::default()
-        }
-    }
-}
-
-impl<'a> From<&'a str> for Spans<'a> {
-    fn from(s: &'a str) -> Spans<'a> {
-        Spans {
-            content: vec![Span::from(s)],
-            ..Spans::default()
-        }
-    }
-}
-
-impl<'a> From<Vec<Span<'a>>> for Spans<'a> {
-    fn from(spans: Vec<Span<'a>>) -> Spans<'a> {
-        Spans {
-            content: spans,
-            ..Spans::default()
-        }
-    }
-}
-
-impl<'a> From<Span<'a>> for Spans<'a> {
-    fn from(span: Span<'a>) -> Spans<'a> {
-        Spans {
-            content: vec![span],
-            ..Spans::default()
-        }
-    }
-}
-
-impl<'a> From<Spans<'a>> for String {
-    fn from(line: Spans<'a>) -> String {
-        line.content.iter().fold(String::new(), |mut acc, s| {
-            acc.push_str(s.content.as_ref());
-            acc
-        })
-    }
-}
-
 /// A string split over multiple lines where each line is composed of several clusters, each with
 /// their own style.
 ///
@@ -513,7 +370,7 @@ impl<'a> Text<'a> {
     ///
     /// text.reset_style();
     /// for line in &text.lines {
-    ///     for span in &line.content {
+    ///     for span in line.spans() {
     ///         assert_eq!(Style::reset(), span.style);
     ///     }
     /// }
diff --git a/src/text/line.rs b/src/text/line.rs
index c1997fe..c78d781 100644
--- a/src/text/line.rs
+++ b/src/text/line.rs
@@ -1,9 +1,11 @@
 #![allow(deprecated)]
 use super::{Span, Spans, Style};
+use crate::layout::Alignment;
 
 #[derive(Debug, Clone, PartialEq, Default, Eq)]
 pub struct Line<'a> {
-    pub spans: Vec<Span<'a>>,
+    spans: Vec<Span<'a>>,
+    pub alignment: Option<Alignment>,
 }
 
 impl<'a> Line<'a> {
@@ -66,14 +68,67 @@ impl<'a> Line<'a> {
     /// ]);
     ///
     /// line.reset_style();
-    /// assert_eq!(Style::reset(), line.spans[0].style);
-    /// assert_eq!(Style::reset(), line.spans[1].style);
+    /// assert_eq!(Style::reset(), line.spans()[0].style);
+    /// assert_eq!(Style::reset(), line.spans()[1].style);
     /// ```
     pub fn reset_style(&mut self) {
         for span in &mut self.spans {
             span.reset_style();
         }
     }
+
+    /// Returns the contained [`Span`]s.
+    ///
+    /// ## Examples
+    ///
+    /// ```rust
+    /// # use ratatui::text::{Span, Line};
+    /// # use ratatui::style::{Color, Style, Modifier};
+    /// let mut line = Line::from("Hello");
+    ///
+    /// assert_eq!("Hello", line.spans()[0].content);
+    /// ```
+    pub fn spans(&self) -> &Vec<Span<'a>> {
+        &self.spans
+    }
+
+    /// Returns the contained [`Span`]s mutably.
+    ///
+    /// ## Examples
+    ///
+    /// ```rust
+    /// # use std::borrow::Cow;
+    /// # use ratatui::text::{Span, Line};
+    /// # use ratatui::style::{Color, Style, Modifier};
+    /// let mut line = Line::from("Hello");
+    /// line.spans_mut().push(Span::from("!"));
+    ///
+    /// assert_eq!("Hello!", String::from(line));
+    /// ```
+    pub fn spans_mut(&mut self) -> &mut Vec<Span<'a>> {
+        &mut self.spans
+    }
+
+    /// Sets the target alignment for this line of text.
+    /// Defaults to: [`None`], meaning the alignment is determined by the rendering widget.
+    ///
+    /// ## Examples
+    ///
+    /// ```rust
+    /// # use std::borrow::Cow;
+    /// # use ratatui::layout::Alignment;
+    /// # use ratatui::text::{Span, Line};
+    /// # use ratatui::style::{Color, Style, Modifier};
+    /// let mut line = Line::from("Hi, what's up?");
+    /// assert_eq!(None, line.alignment);
+    /// assert_eq!(Some(Alignment::Right), line.alignment(Alignment::Right).alignment)
+    /// ```
+    pub fn alignment(self, alignment: Alignment) -> Self {
+        Self {
+            alignment: Some(alignment),
+            ..self
+        }
+    }
 }
 
 impl<'a> From<String> for Line<'a> {
@@ -90,7 +145,10 @@ impl<'a> From<&'a str> for Line<'a> {
 
 impl<'a> From<Vec<Span<'a>>> for Line<'a> {
     fn from(spans: Vec<Span<'a>>) -> Self {
-        Self { spans }
+        Self {
+            spans,
+            ..Default::default()
+        }
     }
 }
 
@@ -117,6 +175,7 @@ impl<'a> From<Spans<'a>> for Line<'a> {
 
 #[cfg(test)]
 mod tests {
+    use crate::layout::Alignment;
     use crate::style::{Color, Modifier, Style};
     use crate::text::{Line, Span, Spans};
 
@@ -210,4 +269,13 @@ mod tests {
         let s: String = line.into();
         assert_eq!("Hello, world!", s);
     }
+
+    #[test]
+    fn test_alignment() {
+        let line = Line::from("This is left").alignment(Alignment::Left);
+        assert_eq!(Some(Alignment::Left), line.alignment);
+
+        let line = Line::from("This is default");
+        assert_eq!(None, line.alignment);
+    }
 }
diff --git a/src/text/spans.rs b/src/text/spans.rs
index 1e20eed..65322bb 100644
--- a/src/text/spans.rs
+++ b/src/text/spans.rs
@@ -1,5 +1,8 @@
 #![allow(deprecated)]
+
 use super::{Span, Style};
+use crate::layout::Alignment;
+use crate::text::Line;
 
 /// A string composed of clusters of graphemes, each with their own style.
 ///
@@ -79,6 +82,24 @@ impl<'a> Spans<'a> {
             span.reset_style();
         }
     }
+
+    /// Sets the target alignment for this line of text.
+    /// Defaults to: [`None`], meaning the alignment is determined by the rendering widget.
+    ///
+    /// ## Examples
+    ///
+    /// ```rust
+    /// # use std::borrow::Cow;
+    /// # use ratatui::layout::Alignment;
+    /// # use ratatui::text::{Span, Spans};
+    /// # use ratatui::style::{Color, Style, Modifier};
+    /// let mut line = Spans::from("Hi, what's up?").alignment(Alignment::Right);
+    /// assert_eq!(Some(Alignment::Right), line.alignment)
+    /// ```
+    pub fn alignment(self, alignment: Alignment) -> Line<'a> {
+        let line = Line::from(self);
+        line.alignment(alignment)
+    }
 }
 
 impl<'a> From<String> for Spans<'a> {
diff --git a/src/widgets/calendar.rs b/src/widgets/calendar.rs
index 4538610..8198fe8 100644
--- a/src/widgets/calendar.rs
+++ b/src/widgets/calendar.rs
@@ -146,16 +146,16 @@ impl<'a, S: DateStyler> Widget for Monthly<'a, S> {
 
         // go through all the weeks containing a day in the target month.
         while curr_day.month() as u8 != self.display_date.month().next() as u8 {
-            let mut line = Spans::from(Vec::with_capacity(14));
+            let mut spans = Vec::with_capacity(14);
             for i in 0..7 {
                 // Draw the gutter. Do it here so we can avoid worrying about
                 // styling the ' ' in the format_date method
                 if i == 0 {
-                    line.content.push(Span::styled(" ", Style::default()));
+                    spans.push(Span::styled(" ", Style::default()));
                 } else {
-                    line.content.push(Span::styled(" ", self.default_bg()));
+                    spans.push(Span::styled(" ", self.default_bg()));
                 }
-                line.content.push(self.format_date(curr_day));
+                spans.push(self.format_date(curr_day));
                 curr_day += Duration::DAY;
             }
             buf.set_line(area.x, area.y, &spans.into(), area.width);
diff --git a/src/widgets/paragraph.rs b/src/widgets/paragraph.rs
index e89248e..d10399f 100644
--- a/src/widgets/paragraph.rs
+++ b/src/widgets/paragraph.rs
@@ -149,13 +149,12 @@ impl<'a> Widget for Paragraph<'a> {
         }
 
         let style = self.style;
-        let styled = self.text.lines.iter().map(|spans| {
+        let styled = self.text.lines.iter().map(|line| {
             (
-                spans
-                    .content
+                line.spans()
                     .iter()
                     .flat_map(|span| span.styled_graphemes(style)),
-                spans.alignment.unwrap_or(self.alignment),
+                line.alignment.unwrap_or(self.alignment),
             )
         });
 
@@ -608,7 +607,7 @@ mod test {
 
     #[test]
     fn test_render_paragraph_with_styled_text() {
-        let text = Spans::from(vec![
+        let text = Line::from(vec![
             Span::styled("Hello, ", Style::default().fg(Color::Red)),
             Span::styled("world!", Style::default().fg(Color::Blue)),
         ]);
diff --git a/src/widgets/reflow.rs b/src/widgets/reflow.rs
index 4be3ef0..99d6f4a 100644
--- a/src/widgets/reflow.rs
+++ b/src/widgets/reflow.rs
@@ -315,9 +315,8 @@ fn trim_offset(src: &str, mut offset: usize) -> &str {
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::text::{Spans, Text};
-
     use crate::style::Style;
+    use crate::text::{Line, Text};
     use unicode_segmentation::UnicodeSegmentation;
 
     enum Composer {
@@ -331,13 +330,12 @@ mod test {
         text_area_width: u16,
     ) -> (Vec<String>, Vec<u16>, Vec<Alignment>) {
         let text = text.into();
-        let styled_lines = text.lines.iter().map(|spans| {
+        let styled_lines = text.lines.iter().map(|line| {
             (
-                spans
-                    .content
+                line.spans()
                     .iter()
                     .flat_map(|span| span.styled_graphemes(Style::default())),
-                spans.alignment.unwrap_or(Alignment::Left),
+                line.alignment.unwrap_or(Alignment::Left),
             )
         });
 
@@ -654,9 +652,9 @@ mod test {
     fn line_composer_preserves_line_alignment() {
         let width = 20;
         let lines = vec![
-            Spans::from("Something that is left aligned.").alignment(Alignment::Left),
-            Spans::from("This is right aligned and half short.").alignment(Alignment::Right),
-            Spans::from("This should sit in the center.").alignment(Alignment::Center),
+            Line::from("Something that is left aligned.").alignment(Alignment::Left),
+            Line::from("This is right aligned and half short.").alignment(Alignment::Right),
+            Line::from("This should sit in the center.").alignment(Alignment::Center),
         ];
         let (_, _, wrapped_alignments) =
             run_composer(Composer::WordWrapper { trim: true }, lines.clone(), width);
diff --git a/tests/widgets_paragraph.rs b/tests/widgets_paragraph.rs
index a7351db..f064e37 100644
--- a/tests/widgets_paragraph.rs
+++ b/tests/widgets_paragraph.rs
@@ -29,7 +29,7 @@ fn test_case(paragraph: Paragraph, expected: Buffer) {
 fn widgets_paragraph_renders_double_width_graphemes() {
     let s = "コンピュータ上で文字を扱う場合、典型的には文字による通信を行う場合にその両端点では、";
 
-    let text = vec![Spans::from(s)];
+    let text = vec![Line::from(s)];
     let paragraph = Paragraph::new(text)
         .block(Block::default().borders(Borders::ALL))
         .wrap(Wrap { trim: true });
@@ -84,7 +84,7 @@ fn widgets_paragraph_renders_mixed_width_graphemes() {
 #[test]
 fn widgets_paragraph_can_wrap_with_a_trailing_nbsp() {
     let nbsp: &str = "\u{00a0}";
-    let line = Spans::from(vec![Span::raw("NBSP"), Span::raw(nbsp)]);
+    let line = Line::from(vec![Span::raw("NBSP"), Span::raw(nbsp)]);
     let paragraph = Paragraph::new(line).block(Block::default().borders(Borders::ALL));
 
     test_case(
@@ -143,7 +143,7 @@ const SAMPLE_STRING: &str = "The library is based on the principle of immediate
 
 #[test]
 fn widgets_paragraph_can_wrap_its_content() {
-    let text = vec![Spans::from(SAMPLE_STRING)];
+    let text = vec![Line::from(SAMPLE_STRING)];
     let paragraph = Paragraph::new(text)
         .block(Block::default().borders(Borders::ALL))
         .wrap(Wrap { trim: true });
@@ -197,7 +197,7 @@ fn widgets_paragraph_can_wrap_its_content() {
 
 #[test]
 fn widgets_paragraph_works_with_padding() {
-    let text = vec![Spans::from(SAMPLE_STRING)];
+    let text = vec![Line::from(SAMPLE_STRING)];
     let paragraph = Paragraph::new(text)
         .block(Block::default().borders(Borders::ALL).padding(Padding {
             left: 2,
@@ -242,8 +242,8 @@ fn widgets_paragraph_works_with_padding() {
         ]),
     );
 
-    let mut text = vec![Spans::from("This is always centered.").alignment(Alignment::Center)];
-    text.push(Spans::from(SAMPLE_STRING));
+    let mut text = vec![Line::from("This is always centered.").alignment(Alignment::Center)];
+    text.push(Line::from(SAMPLE_STRING));
     let paragraph = Paragraph::new(text)
         .block(Block::default().borders(Borders::ALL).padding(Padding {
             left: 2,
@@ -280,8 +280,8 @@ fn widgets_paragraph_can_align_spans() {
     let default_s = "This string will be aligned based on the alignment of the paragraph.";
 
     let text = vec![
-        Spans::from(right_s).alignment(Alignment::Right),
-        Spans::from(default_s),
+        Line::from(right_s).alignment(Alignment::Right),
+        Line::from(default_s),
     ];
     let paragraph = Paragraph::new(text)
         .block(Block::default().borders(Borders::ALL))
@@ -320,7 +320,7 @@ fn widgets_paragraph_can_align_spans() {
 
     let left_lines = vec!["This string", "will override the paragraph alignment"]
         .into_iter()
-        .map(|s| Spans::from(s).alignment(Alignment::Left))
+        .map(|s| Line::from(s).alignment(Alignment::Left))
         .collect::<Vec<_>>();
     let mut lines = vec![
         "This",
@@ -329,7 +329,7 @@ fn widgets_paragraph_can_align_spans() {
         "truncation.",
     ]
     .into_iter()
-    .map(Spans::from)
+    .map(Line::from)
     .collect::<Vec<_>>();
 
     let mut text = left_lines.clone();

@TimerErTim TimerErTim changed the title feat(paragraph)!: add alignments to Spans feat(paragraph)!: add alignments to Line May 23, 2023
@TimerErTim
Copy link
Contributor Author

I have now restored the spans field in Line.

Done in order to enable line scoped alignments for finer grained
styling of widgets.

BREAKING CHANGE: Addition of new field in struct breaks backwards compatibility.
Copy link
Member

@joshka joshka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation LGTM. Everything we've chatted about is resolved. Thanks for the work you've put into this @TimerErTim.

@joshka joshka changed the title feat(paragraph)!: add alignments to Line feat(paragraph): allow Lines to be individually aligned May 25, 2023
@joshka
Copy link
Member

joshka commented May 25, 2023

I renamed the PR so that it will flow through to the commit message / CHANGELOG.
Also this is no longer a breaking change (except for people using the library from git directly)

Here's my suggested commit title / body for the squashed commit that I think captures the main idea and detail of this PR:

Title:

feat(paragraph): allow Lines to be individually aligned

Body:

`Paragraph` now supports rendering each line in the paragraph with a
different alignment (Center, Left and Right) rather than the entire
paragraph being aligned the same. Each line either overrides the
paragraph's alignment or inherits it if the line's alignment is
unspecified.

- Adds `alignment` field to `Line` and builder methods on `Line` and
`Span`
- Updates reflow module types to be line oriented rather than symbol
oriented to take account of each lines alignment
- Adds unit tests to `Paragraph` to fully capture the existing and new
behavior

@joshka joshka merged commit 32e416e into ratatui-org:main May 26, 2023
7 checks passed
@joshka joshka mentioned this pull request Jul 5, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants