fix: resolve 4 bugs in termui - #3592
Conversation
📝 WalkthroughWalkthroughThree example applications receive focused fixes for numeric validation, floating-point percentage rounding, and hexadecimal entity parsing. ChangesExample correctness fixes
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/pomodoro-timer/src/index.tsx`:
- Line 185: Update the percentage calculation in the label expression of the
relevant render method so Number.EPSILON is added to this._value before
multiplying by 100, ensuring boundary values such as 0.145 round to 15%. Add a
regression test covering setValue(0.145) and verify the rendered label displays
15%.
In `@examples/rss-reader/src/index.tsx`:
- Line 30: Update the hexadecimal entity parsing around codePoint to validate
the full captured digit sequence before parsing; when it exceeds eight digits,
return match unchanged, otherwise parse all digits without truncating. Preserve
the existing behavior for valid entities processed by stripMarkup and
readAtomLink.
- Around line 30-31: Update the entity decoding logic around codePoint to
validate that the parsed value is an integer between 0 and 0x10ffff before
calling String.fromCodePoint; return match for non-finite, fractional, negative,
or out-of-range values.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f5522a30-d28d-40d8-ac3c-f2da4be349cb
📒 Files selected for processing (3)
examples/calculator/src/index.tsxexamples/pomodoro-timer/src/index.tsxexamples/rss-reader/src/index.tsx
| const attrs = styleToCellAttrs(this._style); | ||
|
|
||
| const label = this._showLabel ? ` ${Math.round(this._value * 100)}%` : ''; | ||
| const label = this._showLabel ? ` ${Math.round(this._value * 100 + Number.EPSILON)}%` : ''; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Apply Number.EPSILON before scaling the percentage.
setValue allows values such as 0.145. The current expression can keep the result just below 14.5, so the label can render 14% instead of 15%.
Proposed fix
- const label = this._showLabel ? ` ${Math.round(this._value * 100 + Number.EPSILON)}%` : '';
+ const label = this._showLabel ? ` ${Math.round((this._value + Number.EPSILON) * 100)}%` : '';Add a regression test for the 0.145 boundary.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const label = this._showLabel ? ` ${Math.round(this._value * 100 + Number.EPSILON)}%` : ''; | |
| const label = this._showLabel ? ` ${Math.round((this._value + Number.EPSILON) * 100)}%` : ''; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/pomodoro-timer/src/index.tsx` at line 185, Update the percentage
calculation in the label expression of the relevant render method so
Number.EPSILON is added to this._value before multiplying by 100, ensuring
boundary values such as 0.145 round to 15%. Add a regression test covering
setValue(0.145) and verify the rendered label displays 15%.
| return value.replace(/&(#x?[0-9a-fA-F]+|[a-zA-Z]+);/g, (match, entity: string) => { | ||
| if (entity.startsWith('#x')) { | ||
| const codePoint = Number.parseInt(entity.slice(2), 16); | ||
| const codePoint = Number.parseInt(entity.slice(2, 10), 16); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject hexadecimal entities longer than eight digits.
entity.slice(2, 10) truncates the captured entity. It does not enforce an eight-digit limit. For A, the parser reads 00000004 and returns U+0004 instead of U+0041. This can corrupt titles and links processed by stripMarkup and readAtomLink.
If the eight-digit cap is intentional, validate the full digit length and return match when it exceeds eight.
Proposed fix
- const codePoint = Number.parseInt(entity.slice(2, 10), 16);
+ const digits = entity.slice(2);
+ if (digits.length > 8) return match;
+ const codePoint = Number.parseInt(digits, 16);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const codePoint = Number.parseInt(entity.slice(2, 10), 16); | |
| const digits = entity.slice(2); | |
| if (digits.length > 8) return match; | |
| const codePoint = Number.parseInt(digits, 16); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/rss-reader/src/index.tsx` at line 30, Update the hexadecimal entity
parsing around codePoint to validate the full captured digit sequence before
parsing; when it exceeds eight digits, return match unchanged, otherwise parse
all digits without truncating. Preserve the existing behavior for valid entities
processed by stripMarkup and readAtomLink.
| const codePoint = Number.parseInt(entity.slice(2, 10), 16); | ||
| return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
node - <<'NODE'
const codePoint = Number.parseInt('FFFFFFF', 16);
console.log(Number.isFinite(codePoint)); // true
try {
String.fromCodePoint(codePoint);
} catch (error) {
console.log(error.name); // RangeError
}
NODERepository: Karanjot786/TermUI
Length of output: 172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the reported file and surrounding parsing logic.
if [ -f examples/rss-reader/src/index.tsx ]; then
echo "== file exists =="
wc -l examples/rss-reader/src/index.tsx
echo "== relevant lines =="
sed -n '1,80p' examples/rss-reader/src/index.tsx | cat -n
else
echo "examples/rss-reader/src/index.tsx not found"
fd -p 'index.tsx$' .
fi
echo "== entity-related symbols =="
rg -n "parseInt|fromCodePoint|entity|&#|\\?\\?=" examples/rss-reader/src/index.tsx examples/rss-reader/src 2>/dev/null || trueRepository: Karanjot786/TermUI
Length of output: 5093
🌐 Web query:
mdn String.fromCodePoint RangeError valid Unicode scalar value 0xd800 0xdfff 0x10ffff
💡 Result:
The JavaScript method String.fromCodePoint throws a RangeError if a provided argument is not an integer, is less than 0, or is greater than 0x10FFFF [1][2][3]. The range 0x0000 to 0x10FFFF encompasses all valid Unicode code points [1][4]. Notably, the values 0xD800 through 0xDFFF, which correspond to surrogate code units in UTF-16, are technically valid Unicode code points [5]. Consequently, String.fromCodePoint accepts values in the 0xD800–0xDFFF range and will successfully convert them into their corresponding characters [5]. It does not throw a RangeError for these values, despite them not being "Unicode scalar values" (which exclude surrogates) [5]. The method is designed to allow the creation of strings from these values, meaning it can technically produce strings containing unpaired surrogates [5]. RangeError is strictly reserved for values outside the defined Unicode codespace (i.e., < 0 or > 0x10FFFF) or non-integral values [1][2][3].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Not_a_valid_code_point
- 2: https://github.com/mdn/content/blob/main/files/en-us/web/javascript/reference/global_objects/string/fromcodepoint/index.md
- 3: https://tc39.es/ecma262/2025/multipage/text-processing.html
- 4: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/codePointAt
- 5: https://esdiscuss.org/topic/code-points-vs-unicode-scalar-values
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== behavioral probe =="
node - <<'NODE'
function decodeEntities(value) {
const entities = { amp: '&', lt: '<', gt: '>', quot: '"', apos: "'" };
return value.replace(/&(`#x`?[0-9a-fA-F]+|[a-zA-Z]+);/g, (match, entity) => {
if (entity.startsWith('`#x`')) {
const codePoint = Number.parseInt(entity.slice(2, 10), 16);
return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match;
}
if (entity.startsWith('#')) {
const codePoint = Number.parseInt(entity.slice(1), 10);
return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match;
}
return entities[entity.toLowerCase()] ?? match;
});
}
const inputs = ['&`#xFFFFFFF`;', '&`#x110000`;', '&`#55349`;&`#56320`;', '&`#21474`;'];
for (const input of inputs) {
console.log(`${input}:`);
try {
console.log(` -> ${decodeEntities(input)}`);
} catch (error) {
console.log(` -> error: ${error.name}: ${error.message}`);
}
}
NODERepository: Karanjot786/TermUI
Length of output: 351
🌐 Web query:
npm @termuijs/core @termuijs/jsx versions TermUI codePointAt fromCodePoint unicode surrogate
💡 Result:
TermUI is a TypeScript/JavaScript framework for building terminal applications [1]. Its core architecture is split into several packages, primarily @termuijs/core, which provides the foundational rendering engine, input parsing, and layout system, and @termuijs/jsx, which offers a declarative JSX runtime similar to React [2][1][3]. Regarding Unicode, surrogate pairs, and character handling: 1. Unicode and Surrogate Handling: The TermUI codebase has historically faced challenges with multi-byte Unicode characters, such as emojis. A reported bug in the Avatar widget highlighted that accessing strings via direct index (e.g., str) incorrectly splits surrogate pairs, leading to broken character rendering [4]. To address this, developers are advised to use Unicode-aware methods like Array.from or the spread operator [...string] to correctly iterate over code points, or specifically use codePointAt and fromCodePoint to manipulate characters that occupy more than one 16-bit code unit [4]. 2. ANSI Escape Sequences and Text Utilities: The @termuijs/core package includes string utilities such as stringWidth, truncate, and wordWrap [5]. Developers must be cautious when implementing custom text processing, as standard string methods often fail to account for ANSI escape sequences and multi-byte characters [6]. Historically, bugs have occurred where ANSI codes were incorrectly counted toward visual string width or split across lines, causing formatting corruption [6]. 3. Recommended Practices: For terminal-accurate text manipulation in TermUI: - Use specialized utility functions provided by @termuijs/core (e.g., stringWidth) which are designed to handle CJK (wide) characters and ANSI escape sequences correctly [6][5]. - When manual iteration or character extraction is required, rely on Unicode code point iteration (e.g., for...of loops or [...string]) to avoid the pitfalls of UTF-16 surrogate pair splitting [4]. - Be aware of the distinction between character count and display width; many characters (especially CJK or emojis) occupy two terminal columns, which is handled by the framework's internal display width utilities [7][8].
Citations:
- 1: https://github.com/Karanjot786/TermUI
- 2: https://registry.npmjs.org/@termuijs/jsx
- 3: https://github.com/Karanjot786/TermUI/blob/main/docs/choosing-your-api.md
- 4: [Bug] Avatar initials extraction breaks on multi-byte Unicode characters (emojis) #2051
- 5: https://www.pkgstats.com/pkg:`@termuijs/core`
- 6: bug(core): ANSI escape sequence corruption in wordWrap and color bleeding in truncate #2017
- 7: https://term-ui.hexdocs.pm/TermUI.Renderer.DisplayWidth.html
- 8: https://pub.dev/documentation/termui/latest/ui_widgets_display_text/
Validate Unicode code points before calling String.fromCodePoint.
Number.isFinite accepts values above the Unicode codespace, so malformed numeric entities such as � throw RangeError and can abort entity decoding. Return match unless codePoint is a valid integer in the range 0..0x10ffff.
Proposed fix
Fix lines 29 and 34
- return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match;
+ const isCodePoint = Number.isInteger(codePoint) && codePoint >= 0 && codePoint <= 0x10ffff;
+ return isCodePoint ? String.fromCodePoint(codePoint) : match;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const codePoint = Number.parseInt(entity.slice(2, 10), 16); | |
| return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match; | |
| const codePoint = Number.parseInt(entity.slice(2, 10), 16); | |
| const isCodePoint = Number.isInteger(codePoint) && codePoint >= 0 && codePoint <= 0x10ffff; | |
| return isCodePoint ? String.fromCodePoint(codePoint) : match; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/rss-reader/src/index.tsx` around lines 30 - 31, Update the entity
decoding logic around codePoint to validate that the parsed value is an integer
between 0 and 0x10ffff before calling String.fromCodePoint; return match for
non-finite, fractional, negative, or out-of-range values.
1 participant
Description
This PR fixes real bugs found in the codebase:
parseInt: without10, strings like'0x1F'or'08'parse in unintended bases.Number.EPSILONtoMath.round: prevents floating-point drift (e.g.1.005 * 100rounding to 100 instead of 101).trim()to''misses whitespace-only input;.trim().length === 0is explicit.isNaNwithNumber.isNaN: the global version coerces its argument, soisNaN('1')returns false whileNumber.isNaNis strict.Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3591
Summary by CodeRabbit