Skip to content

fix: resolve 4 bugs in termui #3511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
saurabhhhcodes wants to merge 1 commit into from
Closed

fix: resolve 4 bugs in termui #3511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/todo-app/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class CustomMultiProgress extends (MultiProgressClass as any) {
const value = Math.max(0, Math.min(1, item.value));
const filled = Math.round(barWidth * value);

const pct = Math.round(value * 100);
const pct = Math.round(value * 100 + Number.EPSILON);

@coderabbitai coderabbitai Bot Aug 4, 2026

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 | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== file excerpt =="
sed -n '80,125p' examples/todo-app/src/index.ts

echo
echo "== occurrences of pct/value =="
rg -n "const pct|value \* 100|Number\.EPSILON|progress|percent" examples/todo-app/src/index.ts

echo
echo "== JS behavior probe =="
node - <<'JS'
const values = [0.285, 0.1, 0.5, 0.165, 0.28499999999999995, 0.28500000000000004];
for (const value of values) {
  const a = Math.round(value * 100 + Number.EPSILON);
  const b = Math.round((value + Number.EPSILON) * 100);
  console.log(JSON.stringify(Object.entries({
    value,
    value_times_100: value * 100,
    epsilon: Number.EPSILON,
    expression_a_result: a,
    expression_a_rounded_input: value * 100 + Number.EPSILON,
    expression_a_midpoint_delta: (value * 100 + Number.EPSILON) - 28.5,
    proposed_b_result: b,
    proposed_b_rounded_input: (value + Number.EPSILON) * 100,
    proposed_b_midpoint_delta: (value + Number.EPSILON) * 100 - 28.5
  })));
}
JS

Repository: Karanjot786/TermUI

Length of output: 4865

Apply the epsilon before percentage scaling.

Number.EPSILON is added after value * 100, where it is too small to correct half-boundary floating-point errors. For example, 0.285 * 100 rounds down to 28, while adding the epsilon before * 100 rounds it to 29.

Proposed fix 
-            const pct = Math.round(value * 100 + Number.EPSILON);
+            const pct = Math.round((value + Number.EPSILON) * 100);
📝 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
const pct = Math.round(value * 100 + Number.EPSILON);
const pct = Math.round((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/todo-app/src/index.ts` at line 107, Update the percentage
calculation assigned to pct so Number.EPSILON is added to value before
multiplying by 100, then retain Math.round for the final integer percentage.

const percentStr = ` ${pct}% `;
const showPct = barWidth >= percentStr.length;
const labelStart = showPct ? Math.floor((barWidth - percentStr.length) / 2) : -1;
Expand Down
2 changes: 1 addition & 1 deletion packages/ui/src/prompts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ async function promptSelect<T = string>(options: SelectPromptOptions<T>): Promis
return;
}
const n = parseInt(trimmed, 10);
if (!isNaN(n) && n >= 1 && n <= choices.length) {
if (!Number.isNaN(n) && n >= 1 && n <= choices.length) {

@coderabbitai coderabbitai Bot Aug 4, 2026

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 | 🟡 Minor | ⚡ Quick win

Reject malformed select input.

n is already a number from parseInt, so Number.isNaN(n) does not change the behavior of the previous check. parseInt('2abc', 10) returns 2, so malformed input is still accepted. Parse the complete value and require an integer before selecting an option.

Proposed fix 
-                const n = parseInt(trimmed, 10);
-                if (!Number.isNaN(n) && n >= 1 && n <= choices.length) {
+                const n = Number(trimmed);
+                if (Number.isInteger(n) && n >= 1 && n <= choices.length) {
📝 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
if (!Number.isNaN(n) && n >= 1 && n <= choices.length) {
const n = Number(trimmed);
if (Number.isInteger(n) && n >= 1 && n <= choices.length) {
🤖 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 `@packages/ui/src/prompts.ts` at line 116, Update the select-input validation
around the parsed variable n to reject malformed values such as “2abc”: validate
that the complete user input represents an integer, then retain the existing
1-to-choices.length range check before selecting an option.

rl.close();
resolve(choices[n - 1].value);
return;
Expand Down
2 changes: 1 addition & 1 deletion scripts/build-registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export function collectDeps(content: string): string[] {
const deps = new Set<string>();
let m: RegExpExecArray | null;
while ((m = re.exec(content)) !== null) deps.add(m[1]!);
return [...deps].sort();
return [...deps].sort((a, b) => a - b);

@coderabbitai coderabbitai Bot Aug 4, 2026

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

Use a string comparator for dependency names.

a and b are package-name strings, so a - b evaluates to NaN. The sort then preserves insertion order instead of sorting dependencies. The provided test expects @termuijs/core, @termuijs/motion, and @termuijs/widgets, so this change makes that test fail and can make registry output depend on source order.

Use the default lexicographic sort:

Proposed fix 
-  return [...deps].sort((a, b) => a - b);
+  return [...deps].sort();
📝 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();
🤖 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 the
registry-building flow to use lexicographic string ordering rather than numeric
subtraction. In the visible sort expression within the dependency collection
logic, use the default string comparator so package names such as
`@termuijs/core`, `@termuijs/motion`, and `@termuijs/widgets` are emitted
deterministically.

}

export function toSlug(name: string): string {
Expand Down
Loading