tui-textarea icon indicating copy to clipboard operation
tui-textarea copied to clipboard

Add support to set text wrapping

Open brooksvb opened this issue 2 years ago • 10 comments

Closes issue https://github.com/rhysd/tui-textarea/issues/5

Pretty straightforward, just adding a way to set text wrapping on the internal Paragraph widget created when rendering the TextArea.

I tested with the examples, and tested with the search function using the editor example. Text wrapping worked as expected without any apparent interference with the search functionality. Passes tests and linting checks.

Feedback is welcome.

brooksvb avatar Mar 29 '23 20:03 brooksvb

I almost decided to just have a bool for the wrap field, with the assumption that if you want text wrapping, you don't want trim: true, otherwise space at the beginning and end of lines does not get rendered. Instead of making this assumption, I allowed users to directly use the Wrap struct instead, so they can make that decision.

brooksvb avatar Mar 29 '23 22:03 brooksvb

Thank you for trying to make a patch. However this doesn't work because tui-textarea is managing scroll position by itself and it assumes height is equal to number of lines. I think you would find it if you run cargo run --example editor --features=search tui-textarea/README.md in a narrow terminal and scroll down the screen by Ctrl+N.

rhysd avatar Apr 04 '23 11:04 rhysd

I actually just came across this in my own program using my patch. Came back here to comment about it and saw you responded today. Sorry I didn't catch it sooner; I'll look into the scrolling part to understand it and fix the issue.

brooksvb avatar Apr 05 '23 02:04 brooksvb

I've been cooking my brain over this for days and I'm finally starting to see the light.

The behavior I have aimed for is as follows: 1.1. The top row shown in the viewport will always be the start of a line (this has an undesirable side effect, but simplifies some of the logic for now). This was done because the surrounding implementation sort of requires that the top row is set to a line index.

1.2. The cursor line must always fit inside the viewport, including the wrapped lines. This may require scrolling multiple rows at a time until the entire cursor line fits

1.3 I changed the wrap setting back to a boolean, assuming that the user will never want wrap with whitespace trimming. It seems like a really niche use-case and will complicate calculations for line wraps.

1.4 Any changes for wrapping should have nearly no impact on the performance of non-wrapping use-cases

Known issues: 2.1. The calculation used for the number of line wraps a line will result in is not accurate in all cases: In this example, it calculates the line as occupying 2 rows: image

2.2. As a side effect of the earlier point 1, when a line that wraps goes up out of the viewport, it will scroll multiple lines at once. While this seems to make sense for the case of point 1.2, it feels a little more jarring when multiple lines get scrolled for a cursor line that only occupies a single line. I think fixing this will require scrolling the paragraph widget vertically, and knowing how much to scroll.

2.3 On point 1.2, it is probably undesirable to scroll the entire line into view in an application where a user is writing long paragraphs of prose. In some cases I think it is nice, so perhaps the behavior should be an option that can be set. However, to implement this behavior while keeping the cursor on screen, there will need to be calculations done for awareness of which wrapped row the cursor is on within a line. I'm not yet sure I want to implement that in this PR, as it's going to be another chunk of work.

I plan to iron out these issues, and then try to clean up the code to make it as clear as possible. If you have any feedback on what I have so far, I'd appreciate it. I'm particularly concerned about terminology being clear, but maybe that's best left for the end after I make my code as clear and concise as possible.

I will also try writing some tests for my code to validate cursor movements resulting in the correct viewport scroll position.

brooksvb avatar Apr 09 '23 05:04 brooksvb

I wonder if you'd have any thoughts to add at https://github.com/tui-rs-revival/ratatui/issues/174 regarding scrolling from the things you've learnt from this issue?

joshka avatar May 11 '23 09:05 joshka

Hey @brooksvb, are you still working on this? I'd be more than happy to help bringing this over the finish line :slightly_smiling_face:

erak avatar Aug 25 '23 09:08 erak

Hey @brooksvb, are you still working on this? I'd be more than happy to help bringing this over the finish line 🙂

I wanted to finish it but it continued to balloon in complexity and I ran out of steam while the rest of my life got quite complicated and hectic for a while. :sweat_smile:

You're more than welcome to take over to get it the rest of the way!

brooksvb avatar Aug 26 '23 00:08 brooksvb

Hey @brooksvb, are you still working on this? I'd be more than happy to help bringing this over the finish line 🙂

I wanted to finish it but it continued to balloon in complexity and I ran out of steam while the rest of my life got quite complicated and hectic for a while. 😅

You're more than welcome to take over to get it the rest of the way!

Thanks for your reply! It would be very helpful to get a jump start :slightly_smiling_face:. Could you write up a short summary of the current state? Like, things that need to be done, nice-to-haves, known issues etc.

erak avatar Aug 28 '23 16:08 erak

This would be great to have

deepu105 avatar Oct 02 '23 15:10 deepu105

@erak sorry for the late response. It's been hectic. I would suggest reading the code I've written to understand the current state. I tried to leave good comments for the confusing parts, and a decent commit history for "blame" investigation.

This comment https://github.com/rhysd/tui-textarea/issues/5#issuecomment-1819656078 also contains some good background for the task. It seems @rhysd is considering working on this feature at some point and had some ideas, so maybe get input from them if you're planning to work on this.

brooksvb avatar Nov 20 '23 19:11 brooksvb