fix: resolve 4 bugs in termui - #3704
Conversation
📝 WalkthroughWalkthroughThe PR makes three independent one-line fixes. It replaces global numeric validation, adds an explicit decimal radix for tab parsing, and updates confirmation hint selection for truthy defaults. ChangesCalculator validation
Widget tab parsing
Prompt confirmation
Estimated code review effort: 1 (Trivial) | ~5 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ui/src/prompts.ts (1)
72-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression tests for the hint text.
The existing tests in
packages/ui/src/prompts.test.tsverify only"y"and"n"results. They do not inspect the question passed toreadline, so a regression in this condition could pass unnoticed. AssertY/nfortrue,y/Nforfalse, andy/nwhendefaultis omitted.🤖 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 72, Add regression coverage in the existing prompts tests for the question passed to readline: assert the hint is “Y/n” when options.default is true, “y/N” when false, and “y/n” when omitted. Keep the existing “y” and “n” result assertions intact.
🤖 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.
Nitpick comments:
In `@packages/ui/src/prompts.ts`:
- Line 72: Add regression coverage in the existing prompts tests for the
question passed to readline: assert the hint is “Y/n” when options.default is
true, “y/N” when false, and “y/n” when omitted. Keep the existing “y” and “n”
result assertions intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fdc42c0e-035e-49de-b3ff-96751d0adb6c
📒 Files selected for processing (3)
examples/calculator/src/index.tsxexamples/widget-gallery/src/index.tspackages/ui/src/prompts.ts
Description
This PR fixes real bugs found in the codebase:
parseInt: without10, strings like'0x1F'or'08'parse in unintended bases.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.x === trueis equivalent tox(andx === falseto!x), and shorter to read.Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3703
Summary by CodeRabbit