g.wrapString sometimes removing last character

[retcurve]

g.wrapString is removing the last character if the character before it is one of the canSplitAfter characters, eg . - or ,
This happens no matter how wide the wrap width is. For example -

g.wrapString('test.a', 100) returns test. and not test.a

Multiple characters after the split character work fine (g.wrapString('test.aa', 100) returns test.aa) as do spaces (g.wrapString('test a', 100) returns test a)

[bobrippling]

I can't see any recent changes to wrapString, @gfwilliams possibly a pre-existing bug?

[gfwilliams]

Thanks for the report - yes, I doubt this is something that new - it probably came in when the word-splitting got added.

[rogueeve575]

Not a maintainer, but this line seems to be related:
https://github.com/espruino/Espruino/blob/d6d1d73b28cd10ec01ca0bc0f7de81ee5e9d0d2e/libs/graphics/jswrap_graphics.c#L2545C1-L2545C7

Consider what happens when we run the test case:

• We iterate over the chars "test". That Big If Statement in the middle of the loop isn't executing, so we are just adding to wordWidth.
• When we hit the '.', the bottom of the loop sets canSplitAfter = true.
• Next the 'a' has canSplitAfter set so we copy in "test." to currentLine.
• Because the Big If Statement ran and ch is 'a', not 0, we continue, and never set endOfText.
• The while loop breaks because jsvStringIteratorHasChar() is false and we never set endOfText. So we exit the loop and nothing ever appends the final 'a'.

This doesn't happen with other strings like "testab"; since canSplitAfter is never set, these continue down the bottom of the loop when parsing the last char, and set endOfText. Then they can parse one final char PAST the end of the string (ch = -1), and copy in that last piece.

My instinct is to refactor this function more, but one possible minimal fix is to add another copy of the line that sets endOfText just before the continue:

	if (ch!=0) {
		if (!jsvStringIteratorHasChar(&it)) endOfText=true;
		continue; // allow us to handle images next
	}

...after applying this patch, "test.a" is parsed correctly and tests/test_graphics_wrapString.js still passes (at least, as good as it did before, one "Talmannen Kevin McCarthy" test with all the UTF8 fails out of the box without any patches; maybe that should be looked into too at some point).

[gfwilliams]

Thanks for looking into this! That's great - patch applied.

The whole wrapString function is a bit of a nightmare - I've already rewritten it once, but it's made particularly painful by it having to handle UTF8 (in some builds, not all) and the in-string bitmaps, and then the possibility that it'll have to split strings in the middle that wouldn't normally be split.

Do you think you could post up an example of the UTF8 that fails?

If you think you can come up with something tidier that works better then I'm all for it. It's been such a pain to get right though that if it turns out it's just another one-line change to fix UTF8 I'd be inclined to do that, as the current nasty solution has at least been used by thousands of people on Bangle.js so far and appears to almost be there.