-
Notifications
You must be signed in to change notification settings - Fork 258
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
refactor: visitor method to process paragraph wrapped text #956
base: main
Are you sure you want to change the base?
refactor: visitor method to process paragraph wrapped text #956
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #956 +/- ##
=====================================
Coverage 91.9% 91.9%
=====================================
Files 61 61
Lines 15723 15728 +5
=====================================
+ Hits 14456 14461 +5
Misses 1267 1267 ☔ View full report in Codecov by Sentry. |
Can you please explain the use case for this in a bit more depth and show how this PR solves the problem? This might be a purgatory thing, not because of anything that you've done, but because the public interface around text wrapping needs to be redesigned and adding more public methods will make that difficult. The hassle with it is that the text wrapping problem is much larger than you'd expect. See #293 for details. |
If I were to add some code to the examples of user input editing which also handles text wrapping (unlike the current example), would that be helpful? I suppose a separate example to complement |
Sure thing (bear in mind that the user input example that hasn't really been touched up recently. You might consider looking at some ideas from My primary concern with this PR is that there isn't an example of any external calling code of the new public methods, so it's difficult to know exactly which problem you're solving with this. Perhaps start with the narrative if that would help? |
I think providing an example should be the best way to see, explain and discuss my reasoning. I'll make this new example, update the PR and we can take it from there. |
Marked this as draft while waiting on more info on the use case. |
I will get round to this, just a little short of time at the moment! By the way, I don't believe this line is necessary in your latest code (had a merge issue with it):
From: Might be wrong, but because |
Thanks. I think you're right that the implementation is wrong. |
I recently wrote an edit text feature for wrapped paragraph text based on ratatui. I found that I needed a public interface to visit the wrapped text to handle the edit mode cursor placement logic consistent with text rendering.
The following pull request shows what I had to do to make this possible - I also needed the helper method get_line_offset as a bit of auxiliary public functionality in some of my logic.
Is this something you would consider taking across, or shall I remain in purgatory on this with a fork?