Reuse soft-wrap infrastructure in :reflow #11738
Conversation
rhogenson
force-pushed
the
reflow
branch
2 times, most recently
from
cfb6730 to
fd96796
Compare
|
We want to move away from textwrap to our own wrapping system. That will allow us to implement these kind of thjngs without resorting to hacks. |
|
In particular see #3332 (comment) |
|
Makes sense. Let me see if I can reuse the existing softwrap implementation instead of textwrap. |
rhogenson
force-pushed
the
reflow
branch
2 times, most recently
from
900497c to
e66dc93
Compare
|
I integrated the reflow logic into DocumentFormatter so that we can reuse the existing soft-wrap logic. This does mean that :reflow always preserves existing newlines, so it no longer allows reflowing out to a larger width, but it also makes it more consistent and probably accidentally addresses #2419 |
rhogenson
force-pushed
the
reflow
branch
2 times, most recently
from
5038a9c to
c7fc362
Compare
the-mikedavis
changed the title
| continue_comments: Vec::from( | ||
| doc.language_config() | ||
| .and_then(|config| config.comment_tokens.as_deref()) | ||
| .unwrap_or(&[]), | ||
| ), |
There was a problem hiding this comment.
Hm... I'm wondering if it's worth it to just provide an Option<&'a [&str]> instead of a Vec<&str> to avoid the allocation.
I mean the clone isn't that expensive so it should be fine as well.
Just my two cents 👀
There was a problem hiding this comment.
It's a little inconvenient to have TextFormat borrow from doc. For example, having TextFormat borrow from doc breaks this code:
helix/helix-term/src/commands.rs
Line 1762 in a0bd39d
Since it's also using doc mutably in other places, having text_format borrow doc immutably causes a compile error. It's not obvious to me how to rewrite that code to avoid the issue.
Languages should typically only have 1 or 2 comment prefixes, so I think a small clone is worth it here to avoid lifetime hell.
There was a problem hiding this comment.
Fair, thank you for the insight 👍
I changed :reflow to use the DocumentFormatter object instead of the textwrap crate. This allows using the same logic for soft wrap as for :reflow. Because the logic is the same as for soft wrap, we end up preserving all existing newlines, so it's more like "wrap" than reflow, but I think this behavior makes sense anyway to avoid extraneous diffs.
|
I fixed some bugs in the implementation, but there's still one big issue remaining: it breaks URLs across lines since soft-wrap currently wraps at punctuation. I think that's being tracked as a separate issue though (#11428) |
This uses the single-line comment prefixes from the language config, so it should be able to handle different languages in a robust way. The logic is fairly simple, and doesn't handle block comments, for example.
Fixes #3332, #3622
Thanks for all your work on this editor, it is a joy to use.