fix: resolve 4 bugs in termui - #3583
Conversation
github-actions
Bot
added
type:bug
📝 WalkthroughWalkthroughThe pull request updates the form shortcut condition, manages the weather refresh interval through ChangesForm shortcut handling
Weather refresh scheduling
TreeSelect value comparison
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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: 2
🤖 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 condition in the keyboard event handler to use prefix
negation for event.ctrl, so the confirmation modal opens for plain “c” while
Ctrl+C remains handled by the quit branch.
In `@packages/ui/src/TreeSelect.ts`:
- Around line 185-186: Update _valuesEqual so sortedA and sortedB both use the
same local typed comparator suitable for the array element type; remove numeric
subtraction and the comparator-less sort, ensuring equal string permutations are
ordered identically before comparison.
🪄 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: 823c7d03-408a-4fe2-ac96-2989779da2cc
📒 Files selected for processing (3)
examples/forms-and-validation/src/index.tsxexamples/weather/src/index.tsxpackages/ui/src/TreeSelect.ts
| } | ||
|
|
||
| if (event.key === 'c' && event.ctrl === false) { | ||
| if (event.key === 'c' && event.ctrl !) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file="examples/forms-and-validation/src/index.tsx"
if rg -nP "event\.key === 'c' && event\.ctrl\s*!" "$file"; then
echo "Incorrect postfix assertion remains"
exit 1
fi
rg -nF "event.key === 'c' && !event.ctrl" "$file"Repository: Karanjot786/TermUI
Length of output: 245
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file="examples/forms-and-validation/src/index.tsx"
sed -n '100,140p' "$file" | cat -n
echo "--- TypeScript parser behavior for event.ctrl ! vs !event.ctrl ---"
node - <<'JS'
const exprs = ["event.ctrl !", "!event.ctrl"];
for (const expr of exprs) {
try {
const ast = parser = new Function("");
} catch { }
}
console.log("Node is not needed; use TypeScript parser semantics:", {
"event.ctrl !": "postfix non-null assertion expression equals event.ctrl",
"!event.ctrl": "logical not of event.ctrl"
});
JS
# Use a read-only behavior probe that parses representative TypeScript snippets without running repo code.
node - <<'JS'
const ts = require.resolve('typescript');
const tsx = require(ts);
const samples = [
{ code: "if (x && event.ctrl !) {}", title: "event.ctrl !" },
{ code: "if (x && !event.ctrl) {}", title: "!event.ctrl" },
];
for (const {code, title} of samples) {
const sf = tsx.createSourceFile('probe.ts', code, tsx.ScriptTarget.Latest, true, tsx.ScriptKind.TS);
const cond = sf.statements[0].expression;
console.log(title, {
leftOperator: cond.operator,
right: cond.right.getText(sf),
rightOperator: cond.right.operator,
rightArgumentsLength: cond.right.arguments?.length || 0
});
}
JSRepository: Karanjot786/TermUI
Length of output: 2792
Use prefix negation for event.ctrl.
event.ctrl ! is a postfix non-null assertion, so it still only checks whether Control is pressed. Use !event.ctrl to open the confirmation modal on plain c while keeping Ctrl+C handled by the quit branch.
Proposed fix
- if (event.key === 'c' && event.ctrl !) {
+ if (event.key === 'c' && !event.ctrl) {📝 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.
| if (event.key === 'c' && event.ctrl !) { | |
| if (event.key === 'c' && !event.ctrl) { |
🤖 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
condition in the keyboard event handler to use prefix negation for event.ctrl,
so the confirmation modal opens for plain “c” while Ctrl+C remains handled by
the quit branch.
Source: Coding guidelines
| const sortedA = [...a].sort((a, b) => a - b); | ||
| const sortedB = [...b].sort(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 '_valuesEqual|sort\(' packages/ui/src/TreeSelect.ts
while IFS= read -r tsconfig; do
npx tsc --noEmit --pretty false -p "$tsconfig"
done < <(fd -HI '^tsconfig(\..*)?\.json$' .)Repository: Karanjot786/TermUI
Length of output: 947
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'List TS config files:\n'
fd -HI '^tsconfig(\..*)?\.json$' . || true
printf '\nTreeSelect excerpt:\n'
sed -n '160,195p' packages/ui/src/TreeSelect.ts
printf '\nCheck TypeScript availability:\n'
npm exec -- tsc -v || true
printf '\nPackages UI tsconfig references:\n'
find . -path '*packages/ui*' -name 'tsconfig*.json' -print -exec sed -n '1,220p' {} \;
printf '\nRun TypeScript check for packages/ui/src/TreeSelect.ts with available config:\n'
for tsconfig in $(fd -HI '^tsconfig(\..*)?\.json$' .); do
echo "--- $tsconfig ---"
npx tsc --noEmit --pretty false -p "$tsconfig" 2>&1 | awk '/packages\/ui\/src\/TreeSelect\.ts/ || /TS2362/ || /Found [0-9]+ error/'
doneRepository: Karanjot786/TermUI
Length of output: 4472
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- packages/ui/tsconfig.json ---\n'
cat packages/ui/tsconfig.json
printf '\n--- packages/ui/tsconfig.typecheck.json ---\n'
cat packages/ui/tsconfig.typecheck.json
printf '\n--- packages/ui/src/tsconfig related files ---\n'
fd -HI '^tsconfig(\..*)?\.json$' packages/ui/src || true
printf '\n--- Check all TS config files for TS2362/Node errors involving TreeSelect.ts ---\n'
for tsconfig in $(fd -HI '^tsconfig(\..*)?\.json$' .); do
echo "### $tsconfig"
output="$(npx tsc --noEmit --pretty false -p "$tsconfig" 2>&1 || true)"
if printf '%s\n' "$output" | grep -E 'packages/ui/src/TreeSelect\.ts|TS2362|Found [0-9]+ error' >/dev/null 2>&1; then
printf '%s\n' "$output" | grep -E 'packages/ui/src/TreeSelect\.ts|TS2362|Found [0-9]+ error' -C 2 || true
else
echo "no TreeSelect TS2362 errors reported"
fi
doneRepository: Karanjot786/TermUI
Length of output: 247
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- packages/ui/tsconfig.json ---'
cat packages/ui/tsconfig.json
printf '\n--- packages/ui/tsconfig.typecheck.json ---\n'
cat packages/ui/tsconfig.typecheck.json
printf '%s\n' '--- packages/ui/src/tsconfig related files ---'
fd -HI '^tsconfig(\..*)?\.json$' packages/ui/src || true
printf '\n--- Check all TS config files for TS2362/Node errors involving TreeSelect.ts ---\n'
for tsconfig in $(fd -HI '^tsconfig(\..*)?\.json$' .); do
printf '%s\n' "### $tsconfig"
output="$(npx tsc --noEmit --pretty false -p "$tsconfig" 2>&1 || true)"
if printf '%s\n' "$output" | grep -E 'packages/ui/src/TreeSelect\.ts|TS2362|Found [0-9]+ error' >/dev/null 2>&1; then
printf '%s\n' "$output" | grep -E 'packages/ui/src/TreeSelect\.ts|TS2362|Found [0-9]+ error' -C 2 || true
else
printf '%s\n' "no TreeSelect TS2362 errors reported"
fi
doneRepository: Karanjot786/TermUI
Length of output: 10719
Use one typed comparator for both sorted arrays.
_valuesEqual sorts each array independently before comparing it. Use the same comparator for sortedA and sortedB so equal string permutations do not compare as different. A local typed comparator also avoids string subtraction in strict TypeScript.
Proposed fix
- const sortedA = [...a].sort((a, b) => a - b);
- const sortedB = [...b].sort();
+ const compareValues = (left: string, right: string): number => {
+ const leftNumber = Number(left);
+ const rightNumber = Number(right);
+ if (Number.isFinite(leftNumber) && Number.isFinite(rightNumber)) {
+ return leftNumber - rightNumber;
+ }
+ return left.localeCompare(right);
+ };
+ const sortedA = [...a].sort(compareValues);
+ const sortedB = [...b].sort(compareValues);📝 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 sortedA = [...a].sort((a, b) => a - b); | |
| const sortedB = [...b].sort(); | |
| const compareValues = (left: string, right: string): number => { | |
| const leftNumber = Number(left); | |
| const rightNumber = Number(right); | |
| if (Number.isFinite(leftNumber) && Number.isFinite(rightNumber)) { | |
| return leftNumber - rightNumber; | |
| } | |
| return left.localeCompare(right); | |
| }; | |
| const sortedA = [...a].sort(compareValues); | |
| const sortedB = [...b].sort(compareValues); |
🧰 Tools
🪛 GitHub Actions: CI / 0_build-and-test.txt
[error] 185-185: TypeScript errors TS2362 and TS2363: the operands of an arithmetic operation are not typed as any, number, bigint, or an enum. The @termuijs/ui DTS build failed during 'tsup' / 'bun run build' with exit code 1.
🪛 GitHub Actions: CI / build-and-test
[error] 185-185: TypeScript error TS2362: The left-hand side of an arithmetic operation must be a number-compatible type.
[error] 185-185: TypeScript error TS2363: The right-hand side of an arithmetic operation must be a number-compatible type.
🤖 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/TreeSelect.ts` around lines 185 - 186, Update _valuesEqual so
sortedA and sortedB both use the same local typed comparator suitable for the
array element type; remove numeric subtraction and the comparator-less sort,
ensuring equal string permutations are ordered identically before comparison.
Source: Coding guidelines
1 participant
Description
This PR fixes real bugs found in the codebase:
x === trueis equivalent tox(andx === falseto!x), and shorter to read.parseInt: without10, strings like'0x1F'or'08'parse in unintended bases..sort()coerces elements to strings, so[10, 9, 2]sorts as[10, 2, 9]; numeric comparator sorts correctly.Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3582
Summary by CodeRabbit