[fix] text.wrap now returns non-empty list for one character strings

[Sygmei]

text.wrap returns empty list for one-character strings, this PR fixes this by using <= instead of < for the iterator

[alerque]

Thanks for taking the time to contribute!

This looks reasonable, but can you clarify ... if it was returning the wrong result for one character strings before doesn't that imply it was returning the wrong result all along for all lengths because it was failing to evaluate the last position? Or does the nxt shenanigans mean that isn't true? If the latter, isn't this now looping one too many times for other cases? Perhaps a do ... until loop be more appropriate here.

Also how can we test this so that it is demonstrated to work as expected and we don't break it again?

[Sygmei]

@alerque sorry for yesterday's incomplete PR

I added more content :

• Unit tests that checks that it works with old behaviour
• New unit tests that tests for mono-letters and accented characters
• Fixed old unit tests that did not respect the width limit (for wrap / fill)
• Changed while <= to repeat until as recommended
• wrap now uses %s as a "next word search" since old %W did not work with accented characters

[Sygmei]

length of used on all sorts of = 20
should have jumped instead of putting machines, on same line

[Tieske]

This thing has more problems, and I think the proposed solution is just shifting them (didn't test).

Small test;

local ww = require("pl.text").wrap

local t = [[
hello "world" 'this' -is- a bb ccc dddd test... but wouldn't it pass??? final. word-that-can-be-broken
]]

for _, v in ipairs(ww(t,3)) do print(v) end

Output (without the changes in this PR):

hello
"world"
'this'
-is-
a bb              --> 'bb' on the next line?
ccc dddd          --> 'dddd' on the next line?
test.             --> three dots on this line
.. but
wouldn'           --> the issue @Sygmei is fixing
t it
pass?             --> three ? on this line
?? final.
word-
that-
can-be-           --> broken into 'can-' and 'be-'
broken

[Tieske]

this does remind me of this: #141

[Sygmei]

We can change the separator but %w and %W clearly are not ideal solutions
User might want to keep hyphened-words together as well ?

[alerque]

I agree with @Tieske that there are lots of problems and this is just shifting them, but I'm inclined to think that shifting them has some benefit. As long as we have a naive implementation the closer it works to what it says on the tin the better.

On the other hand I have lots of experience with typesetting and wrapping text context is such a hopelessly complex task I do think this naive implementation needs a disclaimer. In fact it should have been called naive_wrap() or naive_chunk() or something like that.

If we wanted to actually fix this we would have to hook into Unicode data –perhaps by using ICU– to actually determine what were word characters and where breaks are allowed in a locale dependent way. There really isn't any way to fake this and get it right more than a small percentage of the time. Given that we don't want to introduce external dependencies, I'm not sure there is anything for it but to just keep faking it or deprecate this entirely. Which brings us back to "we might as well fake it as best we can".

[Sygmei]

One advantage on splitting on space characters only is that the result is usually closer to what the user might expect.
Splitting on %w breaks unicode characters whereas breaking on space might make the line a little longer than expected (better downside imho)

[Tieske]

I rebased and force-pushed your branch @Sygmei

Added the test from a previous comment as well.

[Tieske]

The only problem left imo is that the function takes the width parameter, and treats it as the minimum. Whereas user expectations probably are that it would be the maximum width.

[Sygmei]

I did not change this behaviour since it was clearly documented :
"It will not break long words, so lines can be over the length to that extent."
Should that behaviour change ?

[Tieske]

I read that in a way that words longer than the limit would extend beyond it (they would not be forcefully broken). But that's different from the current behaviour of a minimum length.

Though that might as well be a different pr.

Opinions @alerque ?

[Tieske]

Added a commit that will honour the width given, and adds an option to forcefully break words longer than width.
This is what (I think) a user would expect from these functions.

It was harder than anticipated actually, it has soo many edge cases... added tests for all of them