Skip to content

fix: resolve 4 bugs in termui - #3412

Open
saurabhhhcodes wants to merge 1 commit into
Karanjot786:mainfrom
saurabhhhcodes:fix/termui-41628
Open

fix: resolve 4 bugs in termui#3412
saurabhhhcodes wants to merge 1 commit into
Karanjot786:mainfrom
saurabhhhcodes:fix/termui-41628

Conversation

@saurabhhhcodes

@saurabhhhcodes saurabhhhcodes commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Description

This PR fixes real bugs found in the codebase:

  • Removed redundant boolean comparison: x === true is equivalent to x (and x === false to !x), and shorter to read.
  • Fixed default sort: .sort() coerces elements to strings, so [10, 9, 2] sorts as [10, 2, 9]; numeric comparator sorts correctly.
  • Added explicit radix to parseInt: without 10, strings like '0x1F' or '08' parse in unintended bases.
  • Added Number.EPSILON to Math.round: prevents floating-point drift (e.g. 1.005 * 100 rounding to 100 instead of 101).

Type of Change

  • Bug fix (non-breaking change fixing an issue)

How Has This Been Tested?

  • Local manual testing

Checklist

  • My code follows the style guidelines
  • I have performed a self-review

Related Issue

Ref: #3411

Summary by CodeRabbit

  • Bug Fixes
    • Improved the clear-form keyboard shortcut handling in the forms and validation example.
    • Fixed hexadecimal HTML entity decoding in the RSS reader to correctly limit parsed values.
    • Improved dependency ordering during the build process for more consistent results.

@github-actions github-actions Bot added type:bug +10 pts. Bug fix. area:examples Example apps. labels Aug 3, 2026
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The changes adjust a form shortcut condition, limit hexadecimal entity parsing to eight digits, and replace default dependency sorting with an explicit comparator.

Changes

Form shortcut handling

Layer / File(s) Summary
Clear-form shortcut condition
examples/forms-and-validation/src/index.tsx
The shortcut now triggers when event.ctrl is falsy.

RSS entity decoding

Layer / File(s) Summary
Hexadecimal entity parsing bound
examples/rss-reader/src/index.tsx
Hexadecimal entity parsing now uses at most eight digits.

Dependency sorting

Layer / File(s) Summary
Dependency sort comparator
scripts/build-registry.ts
collectDeps now sorts dependency strings with a subtraction-based comparator.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: karanjot786

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the changes and marks the bug-fix type, but it omits the required package section and uses Ref instead of Closes for the issue. Add the affected package(s), change the issue reference to Closes #3411, and complete the required checklist and GSSoC sections.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies a fix for four TermUI bugs and follows the required type-and-description format.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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/forms-and-validation/src/index.tsx`:
- Line 125: Update the Ctrl+C handling in the keyboard event handler to remove
the invalid postfix assertion and avoid duplicating the existing Ctrl+C quit
condition. Ensure the clear-form shortcut uses valid syntax and remains
reachable under its intended condition.

In `@examples/rss-reader/src/index.tsx`:
- Line 30: Update the numeric entity handling in decodeEntities so codePoint is
validated with Number.isInteger and constrained to the valid Unicode maximum
0x10ffff before calling String.fromCodePoint; return the original match for
invalid numeric entities to avoid RangeError.
- Line 30: Update the entity-decoding logic around codePoint so it validates the
hexadecimal digit count before parsing, rejecting overlong entities instead of
truncating them. After validation, parse the complete hex body without the
leading “&`#x`” prefix or trailing semicolon using the specified full-range slice.

In `@scripts/build-registry.ts`:
- Line 47: Update the dependency sorting in collectDeps to use string comparison
instead of numeric subtraction; replace the a - b comparator with the default
lexicographic sort or a.localeCompare(b), preserving deterministic ordering for
the returned string[].
🪄 Autofix (Beta)

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: 3d2c09dc-482b-49be-996e-286a2049f13c

📥 Commits

Reviewing files that changed from the base of the PR and between 6c7584e and 265df33.

📒 Files selected for processing (3)
  • examples/forms-and-validation/src/index.tsx
  • examples/rss-reader/src/index.tsx
  • scripts/build-registry.ts

}

if (event.key === 'c' && event.ctrl === false) {
if (event.key === 'c' && event.ctrl !) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and inspect the relevant file/context.
if [ -f examples/forms-and-validation/src/index.tsx ]; then
  echo "== file exists =="
  wc -l examples/forms-and-validation/src/index.tsx
  echo "== relevant lines 105-135 =="
  sed -n '105,135p' examples/forms-and-validation/src/index.tsx | nl -ba -v105
  echo "== event.ctrl occurrences =="
  rg -n "event\.ctrl|setValues|clear|form" examples/forms-and-validation/src/index.tsx
else
  echo "target file missing"
  git ls-files | rg 'examples/forms-and-validation/src/index\.tsx|index\.tsx' || true
fi

# Behavioral probe for TypeScript-like postfix non-null on Event.ctrl.
# TypeScript's event.ctrl is boolean; event.ctrl ! does not invert it.
node - <<'JS'
const values = { foo: { x: 'hi' } };
const cases = [
  { key: 'c', ctrl: false, name: 'plain c' },
  { key: 'c', ctrl: true, name: 'ctrl+c' },
];
for (const event of cases) {
  // Simulate the current condition's result for the TypeScript bool/non-null semantics.
  const current = event.key === 'c' && event.ctrl;
  console.log(JSON.stringify({ event: event.name, eventCtrl: event.ctrl, currentCondition: current }));
}
JS

Repository: Karanjot786/TermUI

Length of output: 291


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== relevant lines 105-135 =="
sed -n '105,135p' examples/forms-and-validation/src/index.tsx | awk '{printf "%5d  %s\n", NR+104, $0}'

echo "== event.ctrl/shortcut occurrences =="
grep -n "event\.ctrl\|key === 'c'\|setValues\|values" examples/forms-and-validation/src/index.tsx || true

echo "== behavioral trace =="
node - <<'JS'
const cases = [
  { key: 'c', alt: false, ctrl: false, name: 'plain c' },
  { key: 'c', alt: false, ctrl: true, name: 'ctrl+c' },
  { key: 'c', alt: true, ctrl: true, name: 'ctrl+alt+c' },
];
for (const event of cases) {
  const current = event.key === 'c' && event.ctrl;
  const fixed = event.key === 'c' && !event.ctrl && event.alt;
  console.log(JSON.stringify({ event: event.name, ctrl: event.ctrl, currentCondition: current, fixedCondition: fixed }));
}
JS

Repository: Karanjot786/TermUI

Length of output: 1886


Use valid syntax instead of a trailing postfix assert.

event.ctrl ! is parsed as event.ctrl ! and needs an operand, so this branch never registers and the clear-form shortcut stays unreachable. The line also tests exactly the same condition as the Ctrl+C quit guard above it.

🤖 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/forms-and-validation/src/index.tsx` at line 125, Update the Ctrl+C
handling in the keyboard event handler to remove the invalid postfix assertion
and avoid duplicating the existing Ctrl+C quit condition. Ensure the clear-form
shortcut uses valid syntax and remains reachable under its intended condition.

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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and inspect the target file and nearby entity-decoding logic.
if [ -f examples/rss-reader/src/index.tsx ]; then
  wc -l examples/rss-reader/src/index.tsx
  sed -n '1,80p' examples/rss-reader/src/index.tsx | cat -n
else
  echo "target file missing"
  fd -a 'index\.tsx$' .
fi

# Check whether String.fromCodePoint raises RangeError at 0x110000 in the sandbox.
node - <<'JS'
const values = [0x10fffe, 0x10ffff, 0x110000, NaN, 1.5];
for (const value of values) {
  let out;
  try {
    out = String.fromCodePoint(value);
  } catch (error) {
    out = `${error.name}: ${error.message}`;
  }
  console.log(`${value} -> ${out}`);
}
JS

Repository: Karanjot786/TermUI

Length of output: 3207


Validate numeric entities before calling String.fromCodePoint.

Number.isFinite(codePoint) still accepts NaN, non-integers, and values above 0x10FFFF, which throw RangeError in decodeEntities(). Use Number.isInteger(codePoint) && codePoint <= 0x10ffff before converting, or return match on invalid numeric entities.

Proposed fix
-      return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match;
+      return Number.isInteger(codePoint) && codePoint <= 0x10ffff
+        ? 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` at line 30, Update the numeric entity
handling in decodeEntities so codePoint is validated with Number.isInteger and
constrained to the valid Unicode maximum 0x10ffff before calling
String.fromCodePoint; return the original match for invalid numeric entities to
avoid RangeError.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Files:\n'
fd -a 'index.tsx$' . | sed 's#^\./##' || true

printf '\nTarget excerpt:\n'
if [ -f examples/rss-reader/src/index.tsx ]; then
  nl -ba examples/rss-reader/src/index.tsx | sed -n '1,80p'
fi

printf '\nSearch decode/parsing context:\n'
rg -n "parseInt|fromCodePoint|entity|&`#x`|U+|codePoint" examples/rss-reader/src/index.tsx || true

printf '\nBehavioral probe for slice(parseInt):/8-digit boundary:\n'
node - <<'JS'
const cases = [
  '&`#x00000041FF`;',
  '&`#x10FFFF`;',
  '&`#x000110000`;',
  '&`#x123456789`;',
];
for (const s of cases) {
  const slice = s.slice(2).slice(0, 8);
  const codePoint = Number.parseInt(slice, 16);
  const full = Number.parseInt(s.slice(2, -1), 16);
  console.log(`${s} first8=${slice} value8=${Number(codePoint).toString(16).toUpperCase()} full=${Number(full).toString(16).toUpperCase()} finite=${Number.isFinite(codePoint)} str=${codePoint <= 0x10FFFF && codePoint >= 0 ? String.fromCodePoint(codePoint) : "non-unicode"}`);
}
JS

Repository: Karanjot786/TermUI

Length of output: 2121


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo 'Target excerpt lines 1-80:'
awk '{printf "%6d  %s\n", NR, $0}' examples/rss-reader/src/index.tsx | sed -n '1,80p'

echo
echo 'Search decode/parsing context:'
grep -nE "parseInt|fromCodePoint|entity|&`#x`|U\+|codePoint" examples/rss-reader/src/index.tsx || true

echo
echo 'Behavioral probe for slice(parseInt)/8-digit boundary:'
node - <<'JS'
const cases = [
  '&`#x00000041FF`;',
  '&`#x10FFFF`;',
  '&`#x000110000`;',
  '&`#x123456789`;',
];
for (const s of cases) {
  const slice = s.slice(2).slice(0, 8);
  const codePoint = Number.parseInt(slice, 16);
  const full = Number.parseInt(s.slice(2, -1), 16);
  console.log(`${s} first8=${slice} value8=${Number(codePoint).toString(16).toUpperCase()} full=${Number(full).toString(16).toUpperCase()} finite=${Number.isFinite(codePoint)} str=${codePoint <= 0x10FFFF && codePoint >= 0 ? String.fromCodePoint(codePoint) : "non-unicode"}`);
}
JS

Repository: Karanjot786/TermUI

Length of output: 4041


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo 'Behavioral probe for actual source slice and intended bounds:'
node - <<'JS'
const cases = [
  ['&`#x00000041FF`;', '8 first hex digits'],
  ['&`#x0000000041FF`;', '8 first hex digits starting after leading x'],
  ['&`#x10FFFF`;', 'max single code point'],
  ['&`#x110000`;', 'first invalid code point'],
  ['&`#xFFFFFFFF`;', 'max 32-bit hex'],
];
for (const [s, note] of cases) {
  const sourceSlice = entity => entity.slice(2, 10);
  const entity = s.slice(1, -1);
  const first8Hex = entity.slice(0, 8);
  const codePoint = Number.parseInt(first8Hex, 16);
  returnCodePoint = Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : "match";
  console.log(`${s} entity=${entity} slice(2,10)=${entity.slice(2,10)} first8Hex=${first8Hex} value=${Number(codePoint).toString(16).toUpperCase()} result=${returnCodePoint}`);
}
JS

Repository: Karanjot786/TermUI

Length of output: 688


🏁 Script executed:

#!/bin/bash
set -euo pipefail

node - <<'JS'
const cases = [
  ['&`#x00000041FF`;', '8 first hex digits'],
  ['&`#x0000000041FF`;', '8 first hex digits after leading `#x`'],
  ['&`#x10FFFF`;', 'max single code point'],
  ['&`#x110000`;', 'first invalid code point'],
  ['&`#xFFFFFFFF`;', 'max 32-bit hex'],
  ['&`#x00000041FF0`;', '9 first hex digits'],
];
function sliceLikeSource(s) {
  const entity = s.slice(1, -1);
  return entity.slice(2, 10);
}
for (const [s, note] of cases) {
  const sliced = sliceLikeSource(s);
  const codePoint = Number.parseInt(sliced, 16);
  const isValidFiniteString = Number.isFinite(codePoint) && codePoint <= 0x10FFFF && codePoint >= 0;
  console.log(JSON.stringify(`${s} -> sliced=${sliced} parse=${sliced ? codePoint.toString(16).toUpperCase() : 'nan'} result=${isValidFiniteString ? String.fromCodePoint(codePoint) : 'match'}`));
}
JS

Repository: Karanjot786/TermUI

Length of output: 496


Do not slice hex digit strings before parsing.

entity.slice(2, 10) removes the & from the value and parses at most 8 characters of the whole &#x...; literal. This decodes &#x00000041FF; as U+0041 instead of rejecting the overlong hex entity, and silently truncates &#x00000041FF0; in the same way. Check the hex digit count first, then parse with Number.parseInt(entity.slice(2, -1), 16).

Proposed fix
-      const codePoint = Number.parseInt(entity.slice(2, 10), 16);
+      const hexDigits = entity.slice(2, -1);
+      if (hexDigits.length > 8) return match;
+      const codePoint = Number.parseInt(hexDigits, 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 entity-decoding
logic around codePoint so it validates the hexadecimal digit count before
parsing, rejecting overlong entities instead of truncating them. After
validation, parse the complete hex body without the leading “&`#x`” prefix or
trailing semicolon using the specified full-range slice.

Comment thread scripts/build-registry.ts
let m: RegExpExecArray | null;
while ((m = re.exec(content)) !== null) deps.add(m[1]!);
return [...deps].sort();
return [...deps].sort((a, b) => a - b);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Confirm the comparator is applied to strings and type-check the project.
rg -n -C 2 'return \[\.\.\.deps\]\.sort' scripts/build-registry.ts
npx tsc --noEmit --pretty false

Repository: Karanjot786/TermUI

Length of output: 5860


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'tracked files matching build registry:\n'
git ls-files | rg '(^|/)build-registry(\.test)?\.ts$|tsconfig\.json|package\.json' || true

printf '\nbuild-registry.ts outline:\n'
ast-grep outline scripts/build-registry.ts 2>/dev/null || true

printf '\nrelevant build-registry.ts lines:\n'
sed -n '1,90p' scripts/build-registry.ts

printf '\npackage typescript reference:\n'
node - <<'JS'
const fs = require('node:fs');
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'));
console.log(JSON.stringify({
  deps: Object.fromEntries(Object.entries(pkg.dependencies||{}).filter(([k]) => k === 'typescript')),
  devDeps: Object.fromEntries(Object.entries(pkg.devDependencies||{}).filter(([k]) => k === 'typescript'))
}, null, 2));
JS

printf '\ndeclarative type-check equivalent for the comparator expression (non-executable):\n'
node - <<'JS'
// Lightweight probe: JS string subtraction behavior for examples present in dependency names.
const examples = ['react', 'react-dom', 'a', 'z', '1', '10'];
for (const a of examples) {
  for (const b of examples) {
    const value = a - b;
    console.log(JSON.stringify({a,b,result:value,number:a - b}));
  }
}
JS

Repository: Karanjot786/TermUI

Length of output: 10683


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'package-manager and bun references:\n'
sed -n '1,80p' package.json
printf '\nlockfile presence:\n'
git ls-files | rg '(^|/)(bun\.lockb?|bun\.lock|package-lock\.json|pnpm-lock\.yaml|yarn\.lock)$' || true

printf '\ntype-check script references:\n'
node - <<'JS'
const fs = require('node:fs');
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'));
console.log(pkg.scripts?.['type-check'] || pkg.scripts?.['tsc'] || pkg.scripts?.['check'] || JSON.stringify(pkg.scripts, null, 2));
JS

printf '\nrun typed compiler against just the file with project TS, suppressing errors unrelated to this file:\n'
PROJECT_TS="$(npx tsc -v 2>/dev/null | grep 'Version ' | sed 's/Version //')"
echo "project TypeScript version: $PROJECT_TS"
tsc --noEmit scripts/build-registry.ts --strict --module esnext --moduleResolution bundler --lib esnext 2>&1 | sed -n '1,80p'

Repository: Karanjot786/TermUI

Length of output: 4585


Use a string comparator for dependency names.

collectDeps returns string[], and a - b makes TypeScript report arithmetic-type errors at scripts/build-registry.ts:42,47. Keep the lexicographic contract by using .sort() (stable order) or a.localeCompare(b).

Proposed fix
-  return [...deps].sort((a, b) => a - b);
+  return [...deps].sort((a, b) => a.localeCompare(b));
📝 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.

Suggested change
return [...deps].sort((a, b) => a - b);
return [...deps].sort((a, b) => a.localeCompare(b));
🤖 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 `@scripts/build-registry.ts` at line 47, Update the dependency sorting in
collectDeps to use string comparison instead of numeric subtraction; replace the
a - b comparator with the default lexicographic sort or a.localeCompare(b),
preserving deterministic ordering for the returned string[].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:examples Example apps. type:bug +10 pts. Bug fix.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant