Adjust string length for fullwidth characters when end is undefined

[kevva]

Fixes #26.
Closes #29.

---

IssueHunt Summary

Referenced issues

This pull request has been submitted to:

• #26: Lose characters when fullwidth characters exist and end is not specified

---

IssueHunt has been backed by the following sponsors. Become a sponsor

[sindresorhus]

@TiagoDanin Can you help review?

[Qix-]

Can we add a few cases where string.normalize() would change the input string, too? I want to make sure that won't break anything.

[kevva]

Good catch, that actually breaks stuff. I wrote the code that included the normalization, but now that I think of it, I don't see why we need it. It'll only modify the string to use a different character than the one inputted. Also, all the tests passes without normalization as well.

'\u006E\u0303test' === '\u006E\u0303test'.normalize();
//=> false

It doesn't break anything, except that if you're going to compare the output with your input some characters might differ.

[Qix-]

I agree, we shouldn't care about it. It's another linear sweep of the string, anyway, so it's a win-win. Let's remove it ^^

[kevva]

I'll do it in a separate PR though since it's unrelated to this.

[TiagoDanin]

LGTM 👍

[TiagoDanin]

I found a big problem.
The support fullwidth characters ( #4, #9) does not work with ANSI combination.

\u001B[31m古古test\u001B[39m does not return the \u001B[31m

---

UPDATE:
Sugestion

if (!astralRegex({exact: true}).test(character) && isFullwidthCodePoint(character.codePointAt())) {
			++visible;
			++begin;
			++stringEnd

			if (typeof end !== 'number') {
				++stringEnd;
			}
		}

This should fix.

Bus this break the test 'supports fullwidth characters', I believe that here "안녕하세" (

slice-ansi/test.js

Line 42 in 9df7d27

t.is(sliceAnsi('안녕하세', 0, 4), '안녕');

) has only 4 (안(c548) 녕(b155) 하(d558) 세(c138)) characters and not 8

[kevva]

@TiagoDanin, could you provide a test example?

[TiagoDanin]

test.failing('fullwidth characters + ANSI codes', t => {
	t.is(sliceAnsi('\u001B[31m古古test\u001B[39m', 3), '\u001B[31m古test\u001B[39m');
	t.is(sliceAnsi('\u001B[31m古古test\u001B[39m', 3, 4), '\u001B[31m古\u001B[39m');
});

[kevva]

@TiagoDanin, that's no bug, you're starting to slice in the middle of a full width character. This works:

t.is(sliceAnsi('\u001B[31m古古test\u001B[39m', 2), '\u001B[31m古test\u001B[39m');
t.is(sliceAnsi('\u001B[31m古古test\u001B[39m', 2, 4), '\u001B[31m古\u001B[39m');

This is also how I'd expect this library to work. If we were to treat full width characters as "single" characters, I'd add an option for it but it's out of scope of this issue.