feat: add configurable ellipsis symbol for text truncation

[soifou]

Allow to customize the ellipsis symbol. I make it optional to not introduce a breaking change in user's theme. Fallback to ....

We should handle the text truncation in the others panes/columns since this is only enabled in queue for now. Also I notice the truncation trigger a bit late (see selected song above).

In any case, what do you think?

[mierak]

What else could the ellipsis be realistically configured to? I have never seen anything other than .... The proposed implementation also does not take into account the length of the configured value when calculating when to display ellipsis.

Good catch with the current implementation being off!

[soifou]

What else could the ellipsis be realistically configured to? I have never seen anything other than ...

This is indeed the convention, but for instance if you have Nerd Fonts, you would want to use a glyph instead ⋯ or …. It subtle, I grant you that.

does not take into account the length of the configured value when calculating when to display ellipsis.

Good catch too. I just wanted to know if you were interested before polishing this a bit.

[mierak]

Sure, go ahead, I would use the nerd font symbols myself if you finish this!

[soifou]

Hmm need some guidance here, is there some padding lying around cells in queue pane?

Because the reported cell width from ratatui needs to be substracted by 4 to be "ellipsized" correctly. Sounds too magic that I feel I'm missing something...

If you want to test directly from master branch, the following hack gets ellipsis trigger correctly:

diff --git a/src/ui/panes/queue.rs b/src/ui/panes/queue.rs
index 81f2937..45d7db1 100644
--- a/src/ui/panes/queue.rs
+++ b/src/ui/panes/queue.rs
@@ -99,7 +99,11 @@ impl Pane for QueuePane {
             .map(|song| {
                 let is_current = status.songid.as_ref().is_some_and(|v| *v == song.id);
                 let columns = (0..formats.len()).map(|i| {
-                    song.as_line_ellipsized(formats[i].prop, widths[i].width.into())
+                    let mut max_len: usize = widths[i].width.into();
+                    if max_len > 10 {
+                        max_len -= 4;
+                    }
+                    song.as_line_ellipsized(formats[i].prop, max_len)
                         .unwrap_or_default()
                         .alignment(formats[i].alignment.into())
                 });

[mierak]

This is most likely due to compounding error with the table having spacing of 1 between columns, so the max width should be something like widths[i].width - i. There is also a mess of areas/blocks in there which could use some cleanup, sorry for that. The table also loses one column from each side on line 155

let table_area = table_block.inner(queue_section);

when it really needs to lose one on the right side to make space for the scrollbar. I think all these add up to the weirdness you are seeing.

The ellipsis used to work, but I think I completely borked it when I introduced the song table format configs.

[soifou]

Thanks for your prompt reply!

I've come up with 65243f3. Might not be ideal but more concise without refactoring the whole logic. Let me know if it fits for you.

[mierak]

This config results in panic on your branch and is fine on master, this should be easily fixable by using saturating_sub() though.

config with panic

```rust song_table_format: [ ( prop: (kind: Property(Other("albumartist")), default: (kind: Property(Artist), default: (kind: Text("Unknown")))), width: "20%", ), ( prop: (kind: Property(Title), default: (kind: Text("Unknown"))), width: "35%", ), ( prop: (kind: Property(Album), style: (fg: "white"), default: (kind: Text("Unknown Album"), style: (fg: "white"))), width: "45%", ), ( prop: (kind: Property(Duration), default: (kind: Text("-"))), width: "5", alignment: Right, ), ], ```

But I am still getting weird behavior especially on the last column with fixed length and the first column sometimes has two columns spacing, which is weird.

Take a look at this solution, maybe its good enough or maybe you can get some ideas from it: (its a patch on top of your branch)

diff with alternative solution

```diff diff --git a/src/ui/panes/queue.rs b/src/ui/panes/queue.rs index bb35725..a3c5e59 100644 --- a/src/ui/panes/queue.rs +++ b/src/ui/panes/queue.rs @@ -32,10 +32,11 @@ use crate::{ }; use log::error; use ratatui::{ + layout::{Flex, Margin}, prelude::{Constraint, Layout, Rect}, style::Stylize, text::Line, - widgets::{Block, Borders, Padding, Row, Table, TableState}, + widgets::{Block, Borders, Row, Table, TableState}, Frame, }; @@ -82,31 +83,49 @@ impl Pane for QueuePane { } = context; let queue_len = queue.len(); + let title = self + .filter + .as_ref() + .map(|v| format!("[FILTER]: {v}{} ", if self.filter_input_mode { "█" } else { "" })); + let table_block = { + let mut b = Block::default().border_style(config.as_border_style().bold()); + if config.theme.show_song_table_header { + b = b.borders(Borders::TOP); + } + if let Some(ref title) = title { + b = b.title(title.clone().blue()); + } + b + }; + + let margin = Margin { + horizontal: 1, + vertical: 0, + }; let header_height = u16::from(config.theme.show_song_table_header); let [table_header_section, mut queue_section] = - *Layout::vertical([Constraint::Min(header_height), Constraint::Percentage(100)]).split(area) - else { - return Ok(()); - }; + Layout::vertical([Constraint::Min(header_height), Constraint::Percentage(100)]).areas(area.inner(margin)); + let mut table_area = table_block.inner(queue_section); + + table_area.width = table_area.width.saturating_sub(1); // make one column space between table and scrollbar + let mut scrollbar_area = table_area; + scrollbar_area.width += 2; // add the two columns that were subtracted from table area self.scrolling_state.set_content_len(Some(queue_len)); - let widths = Layout::horizontal(&self.column_widths).split(table_header_section); - let formats = &config.theme.song_table_format; + let widths = Layout::horizontal(self.column_widths) + .flex(Flex::Start) + .spacing(1) + .split(table_area); - let table_padding = Padding::new(1, 2, 0, 0); - let table_padding_leftright = 3; + let formats = &config.theme.song_table_format; - let ellipsis_len = config.theme.symbols.ellipsis.chars().count(); let table_items = queue .iter() .map(|song| { let is_current = status.songid.as_ref().is_some_and(|v| *v == song.id); let columns = (0..formats.len()).map(|i| { - let mut max_len: usize = widths[i].width.into(); - if max_len - i + table_padding_leftright > ellipsis_len { - max_len -= i + table_padding_leftright; - } + let max_len: usize = widths[i].width.into(); song.as_line_ellipsized(formats[i].prop, max_len, &config.theme.symbols) .unwrap_or_default() @@ -135,31 +154,14 @@ impl Pane for QueuePane { }))) .style(config.as_text_style()) .widths(self.column_widths.clone()) - .block(config.as_header_table_block().padding(table_padding)); + .block(config.as_header_table_block()); frame.render_widget(header_table, table_header_section); } - let title = self - .filter - .as_ref() - .map(|v| format!("[FILTER]: {v}{} ", if self.filter_input_mode { "█" } else { "" })); - let table_block = { - let mut b = Block::default() - .padding(table_padding) - .border_style(config.as_border_style().bold()); - if config.theme.show_song_table_header { - b = b.borders(Borders::TOP); - } - if let Some(ref title) = title { - b = b.title(title.clone().blue()); - } - b - }; let table = Table::new(table_items, self.column_widths.clone()) .style(config.as_text_style()) .row_highlight_style(config.theme.current_item_style); - let table_area = table_block.inner(queue_section); self.table_area = table_area; frame.render_stateful_widget(table, table_area, self.scrolling_state.as_render_state_ref()); frame.render_widget(table_block, queue_section); @@ -171,7 +173,7 @@ impl Pane for QueuePane { self.scrolling_state.set_viewport_len(Some(queue_section.height.into())); frame.render_stateful_widget( config.as_styled_scrollbar(), - queue_section, + scrollbar_area, self.scrolling_state.as_scrollbar_state_ref(), ); ```

In short I tried to mimick the way ratatui's table splits the widths with flex and spacing and then juggled some blocks around. Seems to work nicer this way.

[soifou]

That's strange for the panic because I tested it with a similar config (with fixed duration col) and it was ok. Speaking of duration column, might be good to suggest in default theme the fixed width of 5 cols instead.

And thanks so much for the heavy lifting, I took your solution and reworked it a bit the layout to get a nicer alignment with the scrollbar and removed all the weirdness of adding and removing cols either side. Tested with and without header.

Let me know what you think and thanks again for your patience.

[mierak]

Yeah, you can change the default theme if you want, but dont forget about the one in the docs if you do.

Looks good to me now, we can merge after the small change i noted above. Thanks for taking the time to fix this!

[soifou]

you can change the default theme if you want, but dont forget about the one in the docs if you do.

this involve to update the screenshot i think so i'd rather not to change for now, however this could be the subject of another theme.

Anyway, ready to merge!

[mierak]

done, thanks!