Reflow doesn't work consistently with comments #3332
Comments
|
The current implementation of See also #1937 which is a different feature but may be useful implementation-wise. |
the-mikedavis
added
C-enhancement
|
Thanks for the reference, that does indeed look useful. I basically just want to mimic Vim/NeoVim's behavior here, which I've rarely ever had to think about. Once #1937 lands, I can take a look into reusing that for |
|
This is a known limitation of the current version of |
|
I'd like to add that running Running produces the following result: |
|
I'd like to add that "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Phasellus sed finibus \
sem, luctus dapibus odio. Etiam a sodales quam. Suspendisse auctor, diam eget \
facilisis mattis, justo sem pellentesque quam, a bibendum purus ipsum at justo. \
Etiam odio risus, consectetur nec varius quis, semper in elit. Mauris convallis \
arcu ut elit facilisis, at pretium felis pellentesque. " |
This is an excellent point. I hadn't considered this at all. I think the general plan for making prefix recognition more robust was to work with the textwrap library to expose an API that lets the caller specify what to consider a prefix. I think we could (probably?) allow configuration of the suffix as well. |
|
Is there any issue in textwrap that covers the Helix use cases mentioned here? I didn't see anything that jumped out at me. I could try pushing the ball a little further, since I consider the current |
|
Hello again. I've had some discussion with @mgeisler in this textwrap issue to make some progress. Before we go further, I'd appreciate if a Helix maintainer could read through that issue and give a 👍 to our general path of implementation. |
|
I talked about this a bit with @pascalkuthe and in the long run I think we want to transition to having custom hard-wrapping code in helix-core rather than using the textwrap crate. There's a good chance we'll be able to re-use some of what we already have for soft-wrap and we could tailor the code to the features we need and info we have available like knowing about syntax via tree-sitter and about comment tokens once #4718 is merged. |
I would be interested to hear if this is because the simple wrapping logic is simple enough that it can just be re-implemented? Personally, I would love to see a real editor with support for balanced wrapping 🙂 but that's of course only if you find the feature interesting in the first place. I used LaTeX a ton at my university days, so that is why I'm fascinated by the idea of optimizing wrapping across an entire paragraph. I think it would be quite a unique selling point. If that feature is uninteresting, then I agree that it's easy to implement normal greedy-wrapping by hand. I implemented a quick-and-dirty wrapping function here so I could compare the binary sizes of programs with and without Textwrap. |
|
We
Both contigous hardwrap and softwrapping can not support optimal wrapping (they must be local per defintion) so our softwrap implementation already has it's own wrapping implementation. To cut down on compile time, code size and maintainancr effort it would be ideal to only have one wrapping implementation. I also think it's just kind of odd/inconsistent if softwrap/contiguous hardwrap lead to different results compared to calling the hardwrap command. I also don't really think optimal wrapping makes too much sense for a plaintext editor. Where I think it makes sense is richtext editors or a latex compiler that render to a PDF. I write a ton of LaTeX too but I never really care how my plaintext is wrapped. The wrapping is down by the Compiler later. Same for markdown and other formats. For programming languages textwrap doesn't really work well land something TS based would likely be much better). Linear wrapping is also used by all other comparable editors. So yeah I think linear wrapping (at word boundaries) is good enough for us. Much more important then optimal wrapping is integration with the rest of the editor like comment continuation, wrapping a tree sitter, using non-contigous text as input (for efficency), properly deltified transactions, ... All of these things are vastly simpler to implement (and more importantly maintain) if they are implemented in tree using a single wrapping implementation. |
|
@the-mikedavis @pascalkuthe Thanks for taking a look. I agree that it makes sense to use the same implementation for softwrap and hardwrap. Let me know if you need any help. |
Yes, I totally understand this reasoning! |
|
@pascalkuthe Is this what you are calling "contiguous hard wrap"? |
|
That's #2274 |
|
@the-mikedavis I see. So the only difference between #2274 and #1730 is the former deals with all text and the latter deals only with comments (i.e. detecting that the comment prefix should be added after wrapping)? |
|
They have some overlap but I see them as separate features. #1730 is just for adding in indentation plus the comment token when you hit enter (or equivalent) if you're on a comment line. #2274 would remove the need for you to hit enter in insert mode. It would care about indentation too but is more about balancing the words across the lines automatically while #1730 shouldn't automatically change line contents other than indents and the comment-token |
|
Hi @the-mikedavis , after reading through different issues I am not really sure what plans we have here? I just recently tried helix and evaluated whether I want to switch to it, but one of the main missing features for me is an intuitively working hard-wrapping command. Comparing against for example vs code with the rewrap extension, which just always does what I expect without any need to think about. |
Summary
Most of the time, when I select a comment (e.g. with
x) and run:reflow, the comment prefix (e.g.//) isn't added to the reflowed lines. It seems to happen most when reflowing a single line into multiple lines.Reproduction Steps
I tried creating a basic Go file, the LSP functionality is working correctly in this file (in case that matters).
After selecting the comment and running
:reflow, I expected this to happen:Instead, this happened (ignore the difference in highlighting)
Helix log
~/.cache/helix/helix.log
Platform
Linux
Terminal Emulator
st 0.8.4
Helix Version
helix 22.05 (c5f8a83)
The text was updated successfully, but these errors were encountered: