Repository navigation
refactor(cli): replace comment-backed workarounds and name terminal escapes - #184
Merged
Merged
Conversation
PunGrumpy
added
pkg: cli
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Contributor
|
The latest Docker Doctor scan for this pull request. Learn more about Docker Doctor.
Score: 84 / 100 · 3 issues
Scanned by Docker Doctor for commit |
commit: |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 28 minutes. View limit details
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 |
tryCopy and launchAgent each wrapped a child process in `new Promise` under an `eslint-disable promise/avoid-new`. `once(child, "exit")` from node:events gives the same result: it resolves on exit and rejects on a spawn error. launchAgent's comment also said it resolves true when the agent exits normally. It resolves true on any exit, so the comment now says that and a test pins it. New clipboard tests cover the fallback order and the case where every tool fails.
isCommandAvailable kept an empty catch block alive with a comment that existed only to satisfy no-empty. isExecutableFile returns true or false instead, and the loop reads as a lookup. New tests cover an executable, a file that is not executable and a missing command.
The prompts, the spinner and the score animation wrote raw escape sequences such as `\u001B[?25l` and `\u001B[3A`, with a comment above each to say what it does. terminal-control.ts names them (HIDE_CURSOR, SHOW_CURSOR, CLEAR_TO_LINE_END and cursorUp), so those comments go. The score animation now moves up `whaleLines.length` lines instead of a literal 4. The output is byte-identical. A pseudo-terminal capture of a scan with its spinner, the score animation and two prompts matched the old build.
formatTerminal's last parameter was called unanalyzedCount, with a comment saying it counts the files that could not be read or parsed. A file with nothing to check is also unanalyzed, yet it is not in that count. failedFileCount says what the caller passes.
agentDisplayName special-cased `universal` because, its comment said, getSkillAgentConfig rejects that id. agent-install 0.0.8 accepts it and returns "Universal", so the special case goes. A new install test pins the name.
handoff-payload.ts and diagnostics-dir.ts each defined the same lineSuffix with the same comment. diagnostics-dir.ts now exports it and the handoff prompt imports it.
Four suppressions in the CLI gave no reason, which this repository asks for. The three prompts resolve from inside their keypress listener, and the score animation plays its frames one after another, so the rules stay off at those lines with the reason written inline.
PunGrumpy
force-pushed
the
refactor/cli-workarounds
branch
from
October 23, 2026 02:33
8ddfdca to
2d25d56
Compare
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The comment audit of the CLI found comments that exist to excuse or decode the code next to them. This PR changes the code instead. Behavior and terminal output do not change.
Seven commits:
events.once.tryCopyandlaunchAgentwrappedspawninnew Promiseundereslint-disable promise/avoid-new.once(child, "exit")resolves on exit and rejects on a spawn error, which is the same result.launchAgent's comment claimed it resolves true "when the agent process exits normally". It resolves true on any exit, so the comment now says that and a test pins it.isExecutableFilereplaces an emptycatchwhose only content was a comment to satisfyno-empty.terminal-control.tsexportsHIDE_CURSOR,SHOW_CURSOR,CLEAR_TO_LINE_ENDandcursorUp, and the comments that decoded each raw\u001B[…string ("Hide cursor during prompt", "Move cursor up 4 lines") go. The score animation moves upwhaleLines.lengthlines instead of a literal 4.unanalyzedCountbecomesfailedFileCount. The parameter counts the files that failed to read or parse, not every unanalyzed file, which is why it needed a comment.agentDisplayNamedrops its"universal"special case. Its comment saidgetSkillAgentConfigrejects that id. agent-install 0.0.8 accepts it and returns "Universal".lineSuffixis defined once. It was copied word for word inhandoff-payload.tsanddiagnostics-dir.ts.Not done here: moving the three prompts onto
events.oniteration. It would remove their suppressions, but the prompts have no test harness yet.Related Issues
N/A. Found by a comment audit of the repository.
Checklist
Screenshots (if applicable)
N/A. Terminal output is byte-identical (see below).
Additional Notes
install --agent universalprinting "Universal". Each new test also passed against the old code.scriptran the old and the new build ontest/fixtures/with-errorthrough the spinner, the score animation,askConfirmandaskSelect, and the two captures were identical apart from the scanned directory's name (4 hide-cursor, 5 show-cursor, 24 cursor-up and 17 clear-line sequences each).askMultiSelectonly runs underinstallwithout--agentand was not captured.ultracite check, typecheck and the generated docs are clean. With the reasons in place, oxlint still honors the four directives (removing them brings back threepromise/avoid-newerrors).skip-changeset: no behavior visible to users changes.