feat: wrap lines longer than the terminal width
Previously long lines would be truncated, now instead they are wrapped and a wrap indicator displayed at the front of each line, after the first one.
The wrap indicator and collapse indicators are configurable.
Fixes #277
I have added tests and snapshots for the wrapping behaviour, to demonstrate some aspects of it like the cursor taking up multiple lines, etc. but I haven't updated all the existing snapshots because there is a potentially controversial change:
I opted to widen the "gutter" (where the cursor is displayed) to 2 columns, one for the cursor and one for the wrap indicator. From my perspective showing the wrap indicator is a much better user experience than showing nothing, because otherwise it feels like the cursor behaves inconsistently (since you can't always see what's a wrap without the indicator.)
I wanted to indent the wrapped lines by 1, so that I could make the extra gutter space only when needed, but Paragraph in ratatui does not support this yet. I also tried a suggestion to use the textwrap crate, but that doesn't preserve styled Ratatui spans. There is also this gist that implements custom ratatui wrapping, but that seemed a bit overkill. So widening the gutter by 1 seemed like the lesser evil.
The problem is that this would require me to update all the other snapshots, and I wanted to get your opinion on this before going off and bloating the PR with all of those changes.
As far as the implementation goes, I don't know if this is a good approach. I thought that using ratatui's Paragraph to render this with wrapping would be fairly quick win (which it was), and the rest of the changes are mostly just making sure that line_index is incremented according to the additional screen lines needed for wrapping, and stretching out the cursor, etc.
measure_paragraph_height might seem a bit odd, but I couldn't find a way to have ratatui tell me correctly how many lines will be required to wrap some text, and that's necessary to configure the render rect/area. I tried out the unstable line_count method, but that kept giving me extra newlines for reasons I don't understand.
Edit: I think there is a bug in line_count when a line contains only whitespace, going to report it upstream but it probably means line_count is not a usable option at the moment.
I realised that since there's now space at the front of the line, and the first line doesn't show a wrap indicator, it's possible to move the collapsed indicator there instead now. I didn't actually make this change, but here's what it could look like.
Text wrapping is a great addition!
About adding an exta column to the gutter, I suppose it's possible.
An alternative idea I got was to replace the blue pipe on the left (selection indicator) with ⮓. After all, I suppose you wouldn't be able to select a continuation of a line (ctrl+j, ctrl+k) anyway:
▌@@ -25,9 +25,9 @@ fn copy_hash(r: String) -> Option<Action> {
▌ match &mut state.clipboard {
▌ Some(cb) => {
▌ cb.set_text(r.clone()).map_err(Error::Clipboard)?;
▌- state.display_info("Commit hash copied to clipboar
↪d".to_owned());
▌+ state.display_info("Commit hash copied to clipboar
↪d");
▌ }
Or even doing both?
▌ @@ -25,9 +25,9 @@ fn copy_hash(r: String) -> Option<Action> {
▌ match &mut state.clipboard {
▌ Some(cb) => {
▌ cb.set_text(r.clone()).map_err(Error::Clipboard)?;
▌ - state.display_info("Commit hash copied to clipboar
↪d".to_owned());
▌ + state.display_info("Commit hash copied to clipboar
↪d");
▌ }
One currently annoying thing though, if you double-click to select a whole word in a terminal, say you want to select and copy the commit hash:
▌da1d201 main yada yada
Then this would be selected: ▌da1d201
Termwiz
I've been peeking at perhaps switching over and rewriting the rendering code to use Termwiz entirely instead. It's more low-level, but also feels more intuitive in that widgets can actually have a determined size (Ratatui seems to be all around deciding the size of containers first, and then the widget will have to make use of the size its given).
Navigation
I think there is the assumption in this module when it comes to navigation where it assumes an Item is always a single line. It might be that there's some weird behavior upon navigating if a lot of line-wrapping is in place. Perhaps some preprocessing could be done to accommodate for this? There is already a notion of mapping visual lines to items like:
Line | Item | Content
6 6 ▌modified src/state.rs
7 7 ▌@@ -87,15 +87,7 @@ impl State {…
8 24 ▌@@ -108,9 +100,27 @@ impl State {
9 25 ▌ current_cmd_log: CmdLog::new(),
Found here: https://github.com/altsem/gitu/blob/6b9e124d0c399f93238770d39d1cee1f17aef751/src/screen/mod.rs#L235-L256
Or even doing both?
Leaving the cursor/selection out for a continuation is a good idea.
One currently annoying thing though, if you double-click to select a whole word in a terminal, say you want to select and copy the commit hash:
Then this would be selected:
▌da1d201
Adding the extra gutter column seems to improve this, at least in my terminal.
I think there is the assumption in this module when it comes to navigation where it assumes an Item is always a single line. It might be that there's some weird behavior upon navigating if a lot of line-wrapping is in place. Perhaps some preprocessing could be done to accommodate for this? There is already a notion of mapping visual lines to items like:
The navigation seems to work okay, at least as far as I could tell. What could I try to produce a mismatched line issue? I figured that since the line wrapping is happening only at the render point, that what happens in effect is that the virtual lines (for items) are modelled as single lines (that can never wrap), but at the point they become characters in the terminal then they end up being wrapped, although the model logic doesn't know (or care) that the renderer does that.
One example is when navigating below the visible view, the view should scroll along with the cursor. But if enough lines are wrapped, then it would be delayed.
There is logic in the screen module that makes assumptions about navigation from the size of the view. The size property of Screen.
@jonathanj My intent is to rip out Ratatui and use Wizterm directly. It will mean that the screen module will likely be rewritten.
The idea is to abstract away things using termwiz widgets. And this module will probably manipulate widgets then. And be able to navigate properly when widgets span multiple lines.
Text will actually naturally wrap in a terminal when you just print it out. So this would see some light.
I've already replaced the terminal backend (in ratatui) from crossterm to termwiz. Just need to find a good place to begin the rest
@altsem Thanks for the info! I stumbled across your Wizterm work a couple of weeks ago, and wondered how it might affect something like this.
To be honest, I've had a number of things to deal with IRL the last several weeks, so I haven't been able to dedicate any time to writing code.
I did manage to reproduce the issues you mentioned, and made an attempt to try understand how to fix them, although because of the way Ratatui works I ended up not being able to leverage it's built-in wrapping mechanism much, which also meant needing to manually manage the divergence of screen lines and rendered lines. So a reduction to the complexity here would be most welcome.
Do you think there's any value in me continuing with this PR? The snapshots could presumably still be helpful, as are the configuration. It's likely the screen code that would need to be entirely rewritten, and maybe that's not the worst outcome.
No worries. I'll close this and we'll figure it out in the future rewrite!