-
Notifications
You must be signed in to change notification settings - Fork 192
fix: Make ColumnarMenu suggestions always take 1 line #973
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
base: main
Are you sure you want to change the base?
Conversation
src/menu/menu_functions.rs
Outdated
| /// | ||
| /// If `s` is longer than `max_width`, the resulting string will end in "..." | ||
| /// and have width at most `max_width`. | ||
| pub(crate) fn truncate_no_ansi(s: &str, max_width: usize) -> Cow<str> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's named truncate_no_ansi because I intend to add a truncate_with_ansi helper that truncates strings that do contain ANSI escapes (so it must ignore the ANSI escapes)
| } else { | ||
| suggestion.value.clone() | ||
| }; | ||
| let string = truncate_no_ansi(&suggestion.value, max_string_width); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used the truncation helper here because the original code looks at number of chars rather than width
|
Nice! Works well for me. |
| if let Ok(position) = cursor::position() { | ||
| self.prompt_start_row = position.1; | ||
| self.just_resized = true; | ||
| #[cfg(not(test))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't figure how to mock the Painter struct with mockall so I figured this was easier

Fixes #972
Description
When the longest suggestion in a columnar menu exceeds the screen width, all suggestions will take up more than 1 line. None of the suggestion values will be truncated, and even short suggestion values will be padded to the same length as the longest suggestion. This PR fixes that by truncating values to the screen width.
This PR is just a fix, so I didn't want to change behavior too much, but in the future, we might want to consider basing the padding not on the longest suggestion, but the longest visible suggestion (when there are descriptions). When there aren't descriptions, the number of columns would probably have to be based on the longest suggestion overall, since it might be confusing if you scroll down and the number of columns changes.
Screenshots
Here's an example of how the values get truncated (note that these suggestions have descriptions, they're just not shown because the longest suggestion is too long):
When the longest suggestion isn't too long, you see the descriptions (in this PR, I changed it so that they end in

"..."when truncated):And this is what you get without descriptions:
