util,readline: NFC-normalize strings before getStringWidth · nodejs/node@c82c084 · GitHub

nodejs / node Public

• Notifications You must be signed in to change notification settings
• Fork 36.6k
• Star 120k

• Code
• Issues 637
• Pull requests 761
• Actions
• Projects
• Security and quality
• Insights

Additional navigation options

Commit c82c084

Browse filesBrowse the repository at this point in the historyBrowse files

addaleax

authored and

BethGriggs

committed

util,readline: NFC-normalize strings before getStringWidth

The assumption here is that decomposed characters render like their composed character equivalents, and that working with the former comes with a risk of over-estimating string widths given that we compute them on a per-code-point basis. The regression test added here (한글 vs 한글) is an example of that happening. PR-URL: #33052 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

1 parent 4abc45a commit c82c084
Copy full SHA for c82c084

2 files changed

+19-5Lines changed: 19 additions & 5 deletions

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

• lib/internal/util
  • inspect.js
• test/parallel
  • test-icu-stringwidth.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

‎lib/internal/util/inspect.js‎

Copy file name to clipboardExpand all lines: lib/internal/util/inspect.js

+10-5Lines changed: 10 additions & 5 deletions

Original file line number	Diff line number	Diff line change
@@ -1922,6 +1922,13 @@ function formatWithOptionsInternal(inspectOptions, ...args) {
1922	1922	return str;
1923	1923	}
1924	1924
	1925	+ function prepareStringForGetStringWidth(str, removeControlChars) {
	1926	+ str = str.normalize('NFC');
	1927	+ if (removeControlChars)
	1928	+ str = stripVTControlCharacters(str);
	1929	+ return str;
	1930	+ }
	1931	+
1925	1932	if (internalBinding('config').hasIntl) {
1926	1933	const icu = internalBinding('icu');
1927	1934	// icu.getStringWidth(string, ambiguousAsFullWidth, expandEmojiSequence)
@@ -1931,8 +1938,8 @@ if (internalBinding('config').hasIntl) {
1931	1938	// the receiving end supports.
1932	1939	getStringWidth = function getStringWidth(str, removeControlChars = true) {
1933	1940	let width = 0;
1934		- if (removeControlChars)
1935		- str = stripVTControlCharacters(str);
	1941	+
	1942	+ str = prepareStringForGetStringWidth(str, removeControlChars);
1936	1943	for (let i = 0; i < str.length; i++) {
1937	1944	// Try to avoid calling into C++ by first handling the ASCII portion of
1938	1945	// the string. If it is fully ASCII, we skip the C++ part.
@@ -1952,9 +1959,7 @@ if (internalBinding('config').hasIntl) {
1952	1959	getStringWidth = function getStringWidth(str, removeControlChars = true) {
1953	1960	let width = 0;
1954	1961
1955		- if (removeControlChars)
1956		- str = stripVTControlCharacters(str);
1957		-
	1962	+ str = prepareStringForGetStringWidth(str, removeControlChars);
1958	1963	for (const char of str) {
1959	1964	const code = char.codePointAt(0);
1960	1965	if (isFullWidthCodePoint(code)) {

Collapse file

‎test/parallel/test-icu-stringwidth.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-icu-stringwidth.js

+9Lines changed: 9 additions & 0 deletions

Original file line number	Diff line number	Diff line change
@@ -87,3 +87,12 @@ for (let i = 0; i < 256; i++) {
87	87	assert.strictEqual(getStringWidth(char), 1);
88	88	}
89	89	}
	90	+
	91	+ {
	92	+ const a = '한글'.normalize('NFD'); // 한글
	93	+ const b = '한글'.normalize('NFC'); // 한글
	94	+ assert.strictEqual(a.length, 6);
	95	+ assert.strictEqual(b.length, 2);
	96	+ assert.strictEqual(getStringWidth(a), 4);
	97	+ assert.strictEqual(getStringWidth(b), 4);
	98	+ }

0 commit comments

Comments

0 (0)