Skip to content
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

Subtle bug in soft wrapping? #6829

Closed
sicher opened this issue Apr 20, 2023 · 5 comments · Fixed by #6417
Closed

Subtle bug in soft wrapping? #6829

sicher opened this issue Apr 20, 2023 · 5 comments · Fixed by #6417
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug

Comments

@sicher
Copy link

sicher commented Apr 20, 2023

Hi, I have no insight into how soft wrapping works technically, but I wonder if this is intended behavior. And if it is, if it would make more sense to have helix wrap on whole WORD objects rather than words?

To illustrate, I have the following wrap settings:

text-width = 80
soft-wrap = { enable = true, wrap-at-text-width = true, max-wrap = 25, wrap-indicator = "" }

Now, if I enter the following:

This is a piece of text that will fill more than eighty characters and then wrap,
  expectedly, to the next line.

Here the comma ends up at column 81 (hence why I wonder if this may be a bug?). But moreover, the following space is pushed to the next line. Here I believe it would be nicer if the wrapping would push the WORD "wrap," to the next line:

This is a piece of text that will fill more than eighty characters and then
wrap, expectedly, to the next line.
@pascalkuthe
Copy link
Member

pascalkuthe commented Apr 20, 2023

I am aware of this. The current width limit is increased by one because people we complaining that lines wrapped one character to early thanks to the EOL character taking a width of 1. I have a more correct fix in #6417 but that will take some time to finish

@pascalkuthe pascalkuthe linked a pull request Apr 20, 2023 that will close this issue
@sicher
Copy link
Author

sicher commented Apr 20, 2023

Ok. Just to clarify: If you change the "wrap," to "wrapx", the "wrapx" will be pushed to the second line.

@pascalkuthe
Copy link
Member

pascalkuthe commented Apr 20, 2023

Ah sorry I only scanned the issue those are two seperste issues. I was talking about the "wraps at column 81" which is a bug and fixed by the PR I mentioned. Wrapping at WORD doesn't work well for programming languages. There was a tom of discussion about better wrap strategies in the orginal softwrap PR #5420 (but everyone agreed that it should be left to future work). The conclusions was pretty much:

I don't have time to work on this right now but after the changes to the doc formatter from #6417 land I would be happy o mentor someone to implement that. The code is a bit subtle tough so some experience with rust is needed

@sicher
Copy link
Author

sicher commented Apr 20, 2023

Ok, thanks for the clarification! Yeah, the current implementation totally makes sense as the default.

@kirawi kirawi added C-bug Category: This is a bug A-helix-term Area: Helix term improvements labels Apr 28, 2023
@kj
Copy link

kj commented Jul 15, 2024

@archseer Is this fixed? I just tried this in master with the same settings and the following text (sorry for the silly example):

foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, foo, 

For me that ends up looking like this:

helix_soft_wrap

I understood that the idea was to wrap the words onto the next line and not the space?

Edit: Reading #5420 it looks like this behaviour is due to characters like , being treated as a word boundary. But that hasn't changed (in the context of soft wrapping).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants