-
-
Notifications
You must be signed in to change notification settings - Fork 317
Improve blocks --live rendering
#167
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.
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
Conversation
|
Warning Rate limit exceeded@ryoppippi has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 15 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📥 CommitsReviewing files that changed from the base of the PR and between e32c7255031d6b46ae9145f37b6edbddfef61663 and eaf5617. 📒 Files selected for processing (3)
WalkthroughThe changes introduce advanced terminal output management by enhancing the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LiveMonitor
participant TerminalManager
User->>LiveMonitor: Start live monitoring
LiveMonitor->>TerminalManager: enterAlternateScreen()
LiveMonitor->>TerminalManager: enableSyncMode()
LiveMonitor->>TerminalManager: startBuffering()
loop Each frame
LiveMonitor->>TerminalManager: write(renderedFrame)
LiveMonitor->>TerminalManager: flush()
Note right of LiveMonitor: Frame rate limited (~60fps)
end
LiveMonitor->>TerminalManager: cleanup() (on exit/error)
TerminalManager->>TerminalManager: exitAlternateScreen(), disableSyncMode(), showCursor()
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @bobisme, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request aims to drastically improve the user experience of the blocks --live command by addressing common terminal rendering issues like flickering and visible cursors. It achieves this by leveraging advanced terminal features such as alternate screen buffers and synchronized output modes, alongside implementing a rendering buffer and a frame rate limiter. These changes result in a smoother, more stable, and less CPU-intensive live display.
Highlights
- Improved Terminal Rendering: Implemented buffering, synchronized output (DEC mode 2026), and alternate screen usage to significantly reduce flickering and tearing in the
blocks --livedisplay, especially when the terminal is not focused. - Optimized Performance: Introduced a 60 FPS refresh rate cap to conserve CPU resources during live updates.
- Enhanced User Experience: Ensured the terminal cursor remains hidden during live rendering for a cleaner display.
- Refactored Terminal Utilities: Extended the
TerminalManagerclass with new methods for managing buffering, alternate screens, and synchronized output modes, providing a more robust interface for terminal interactions.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The pull request enhances the live terminal rendering in the blocks command by using the alternate screen, buffering writes, enabling DEC mode 2026, hiding the cursor, and capping the refresh rate. The changes aim to reduce flickering and improve performance, especially when the terminal is not focused. The code introduces a TerminalManager class with methods for managing terminal state, buffering, and synchronized output.
src/commands/_blocks.live.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider extracting the delay logic into a separate function to improve readability and maintainability. This would encapsulate the delay and abort signal handling.
await this.delayWithAbort(MIN_RENDER_INTERVAL - timeSinceLastRender,
abortController.signal
);
private async delayWithAbort(delayTime: number, signal: AbortSignal): Promise<void> {
await delay(delayTime, { signal });
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/_terminal-utils.ts (1)
107-114: Good implementation of alternate screen support.The alternate screen implementation correctly:
- Uses standard ANSI escape sequences
- Disables line wrap in alternate screen to prevent visual artifacts
- Re-enables line wrap when exiting
- Tracks state to avoid redundant operations
One minor suggestion: Consider adding error handling or logging if these operations fail, though terminal operations rarely fail in practice.
Also applies to: 120-127
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 2d862da and 84f955bf56a4c32174d2ba2852f39dfc17f90aa7.
📒 Files selected for processing (2)
src/_terminal-utils.ts(4 hunks)src/commands/_blocks.live.ts(17 hunks)
🧰 Additional context used
🪛 ESLint
src/commands/_blocks.live.ts
[error] 77-77: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 77-77: Unsafe construction of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 121-121: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 122-122: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 122-122: Unsafe member access .signal on an error typed value.
(ts/no-unsafe-member-access)
[error] 132-132: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 132-132: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 132-132: Unsafe member access .cursorTo on an error typed value.
(ts/no-unsafe-member-access)
[error] 133-133: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 133-133: Unsafe member access .eraseDown on an error typed value.
(ts/no-unsafe-member-access)
[error] 135-135: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 135-135: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 135-135: Unsafe member access .yellow on an error typed value.
(ts/no-unsafe-member-access)
[error] 138-138: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 138-138: Unsafe member access .cursorHide on an error typed value.
(ts/no-unsafe-member-access)
[error] 141-141: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 142-142: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 142-142: Unsafe member access .signal on an error typed value.
(ts/no-unsafe-member-access)
[error] 161-161: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 161-161: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 161-161: Unsafe member access .cursorTo on an error typed value.
(ts/no-unsafe-member-access)
[error] 162-162: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 162-162: Unsafe member access .eraseDown on an error typed value.
(ts/no-unsafe-member-access)
[error] 165-165: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 165-165: Unsafe member access .cursorHide on an error typed value.
(ts/no-unsafe-member-access)
[error] 171-171: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 172-172: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 172-172: Unsafe member access .signal on an error typed value.
(ts/no-unsafe-member-access)
[error] 198-198: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 198-198: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 198-198: Unsafe member access .red on an error typed value.
(ts/no-unsafe-member-access)
[error] 200-200: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 200-200: Unsafe member access .error on an error typed value.
(ts/no-unsafe-member-access)
[error] 202-202: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 203-203: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 203-203: Unsafe member access .signal on an error typed value.
(ts/no-unsafe-member-access)
[error] 288-288: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 288-288: Unsafe member access .bold on an error typed value.
(ts/no-unsafe-member-access)
[error] 294-294: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 294-294: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 294-294: Unsafe member access .bold on an error typed value.
(ts/no-unsafe-member-access)
[error] 295-295: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 295-295: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 299-299: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 319-319: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 345-345: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 345-345: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 346-346: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 346-346: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 346-346: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 350-350: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 350-350: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 350-350: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 350-350: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 354-361: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 357-357: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 357-357: Unsafe member access .red on an error typed value.
(ts/no-unsafe-member-access)
[error] 359-359: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 359-359: Unsafe member access .yellow on an error typed value.
(ts/no-unsafe-member-access)
[error] 360-360: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 360-360: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 364-364: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 364-364: Unsafe member access .bold on an error typed value.
(ts/no-unsafe-member-access)
[error] 365-365: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 365-365: Unsafe member access .bold on an error typed value.
(ts/no-unsafe-member-access)
[error] 368-368: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 368-368: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 368-368: Unsafe member access .bold on an error typed value.
(ts/no-unsafe-member-access)
[error] 369-369: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 369-369: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 374-374: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 394-394: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 401-401: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 417-417: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 451-451: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 451-451: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 452-452: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 452-452: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 452-452: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 457-457: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 457-457: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 457-457: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 457-457: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 459-466: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 462-462: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 462-462: Unsafe member access .red on an error typed value.
(ts/no-unsafe-member-access)
[error] 464-464: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 464-464: Unsafe member access .yellow on an error typed value.
(ts/no-unsafe-member-access)
[error] 465-465: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 465-465: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 466-466: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 466-466: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 469-469: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 469-469: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 469-469: Unsafe member access .bold on an error typed value.
(ts/no-unsafe-member-access)
[error] 470-470: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 470-470: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 475-475: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 496-496: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 504-504: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 525-525: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 539-539: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 547-547: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 547-547: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 578-583: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 580-580: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 580-580: Unsafe member access .red on an error typed value.
(ts/no-unsafe-member-access)
[error] 582-582: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 582-582: Unsafe member access .yellow on an error typed value.
(ts/no-unsafe-member-access)
[error] 583-583: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 583-583: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 604-604: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 604-604: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 604-604: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
src/_terminal-utils.ts
[error] 72-72: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 72-72: Unsafe member access .write on an error typed value.
(ts/no-unsafe-member-access)
[error] 92-92: Unexpected any value in conditional. An explicit comparison or type conversion is required.
(ts/strict-boolean-expressions)
[error] 92-92: Unsafe member access .isTTY on an error typed value.
(ts/no-unsafe-member-access)
[error] 93-93: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 93-93: Unsafe member access .write on an error typed value.
(ts/no-unsafe-member-access)
[error] 96-96: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 96-96: Unsafe member access .write on an error typed value.
(ts/no-unsafe-member-access)
[error] 108-108: Unexpected any value in conditional. An explicit comparison or type conversion is required.
(ts/strict-boolean-expressions)
[error] 108-108: Unsafe member access .isTTY on an error typed value.
(ts/no-unsafe-member-access)
[error] 109-109: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 109-109: Unsafe member access .write on an error typed value.
(ts/no-unsafe-member-access)
[error] 109-109: Unsafe member access .enterAlternativeScreen on an error typed value.
(ts/no-unsafe-member-access)
[error] 111-111: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 111-111: Unsafe member access .write on an error typed value.
(ts/no-unsafe-member-access)
[error] 121-121: Unexpected any value in conditional. An explicit comparison or type conversion is required.
(ts/strict-boolean-expressions)
[error] 121-121: Unsafe member access .isTTY on an error typed value.
(ts/no-unsafe-member-access)
[error] 123-123: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 123-123: Unsafe member access .write on an error typed value.
(ts/no-unsafe-member-access)
[error] 124-124: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 124-124: Unsafe member access .write on an error typed value.
(ts/no-unsafe-member-access)
[error] 124-124: Unsafe member access .exitAlternativeScreen on an error typed value.
(ts/no-unsafe-member-access)
🔇 Additional comments (8)
src/_terminal-utils.ts (3)
6-10: LGTM! Synchronized output mode implementation is correct.The DEC mode 2026 escape sequences are the standard way to enable atomic terminal updates, which will help prevent screen tearing during live updates.
65-101: Well-implemented buffering mechanism.The buffering implementation is clean and efficient:
- Proper state management with
useBufferingflag- Atomic writes when flushing with optional sync mode wrapping
- Correctly checks for TTY before applying sync mode escape sequences
This will significantly improve performance when the terminal is not focused, as mentioned in the PR objectives.
172-176: Excellent cleanup implementation.The cleanup method properly restores all terminal states. The order is correct: showing cursor, exiting alternate screen, and disabling sync mode ensures the terminal is left in a clean state.
src/commands/_blocks.live.ts (5)
78-79: Excellent frame rate limiting implementation.The 60fps cap (16ms interval) is a good choice that balances smooth updates with CPU efficiency. The implementation correctly:
- Tracks time since last render
- Skips renders if too soon
- Uses precise timing with
Date.now()This addresses the PR objective of reducing CPU usage during live monitoring.
Also applies to: 116-125
96-104: Proper terminal setup sequence.The setup order is correct:
- Enter alternate screen first (preserves main terminal content)
- Enable sync mode for atomic updates
- Clear screen
- Hide cursor
This sequence ensures a clean start without visual artifacts.
131-139: Significant rendering optimization.Using cursor positioning (
cursorTo(0, 0)) and partial screen erasing (eraseDown) instead of full screen clears is a major improvement. Combined with buffering and sync mode, this will dramatically reduce flicker, especially in Ghostty as mentioned in the PR objectives.The pattern of
startBuffering() -> write operations -> flush()ensures atomic updates.Also applies to: 159-167
141-143: Good improvement to delay handling.Adding the abort signal to all
delay()calls ensures proper cancellation support, preventing potential hanging operations during shutdown.Also applies to: 171-173, 202-204
146-150: Improved error type checking.The more specific error checking for
DOMExceptionorErrorwith name 'AbortError' is more robust than generic error handling. This properly distinguishes between expected abort signals and actual errors.Also applies to: 176-179, 187-190
|
This may be a dupe of #166 |
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the live rendering of blocks in the terminal by reducing flickering and ensuring that the cursor remains hidden during live updates.
- Uses the alternate screen buffer and synchronous updates (DEC mode 2026) to achieve atomic screen updates.
- Implements a frame rate cap (≈60fps) and buffering of terminal writes for smoother rendering.
- Updates terminal utility functions to support buffering and synchronous mode.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/commands/_blocks.live.ts | Refactors live monitoring to use buffered writes, alternate screen, and rate limiting. |
| src/_terminal-utils.ts | Adds buffering and synchronized output mode features for atomic terminal updates. |
src/_terminal-utils.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't you find this escape characters in ansi-escapes ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the codes for alternate screen and similar, but not these
https://github.com/sindresorhus/ansi-escapes/blob/main/base.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least I need to know where these code comes from. if you find a resource, add it as a comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when you done, request a review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a non-standard thing, though a lot of terminals implement it.
https://gist.github.com/christianparpart/d8a62cc1ab659194337d73e399004036
I tested removing it from the PR and it looks like the rest of the changes make far more impact than this one alone.
Should I just remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added that link
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. you can add the escape character if you have a proper reason!
https://gist.github.com/christianparpart/d8a62cc1ab659194337d73e399004036
I think this doc is kind a reasonable. you can add it as a comment.
src/commands/_blocks.live.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also those number should be in const
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/commands/_blocks.live.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious. Is those style fix is happened by eslint or by you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't get eslint --fix to work. it just gave me errors, so I ran prettier with tab formatting. If you know how I can get eslint to fix this, i'll update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not use prettier. if you changes styles too much I cannot review it. if you have any issue for formatting, just ignore it. you can use git commit - n option to ignore git hooks. I'll see how you want to change and do formatting by myself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and I'm sure that CC can solve this formatting issue.
eslint-format and prettier is completely different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you rollback the format issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do it btw..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok check the format correctio/n
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you. sorry about that. my editor is configured to automatically run prettier, so it wrecked the whole thing after I made the code changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no worries, otherwise lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/commands/_blocks.live.ts (1)
236-626: Consider reducing formatting-only changes.While the multiline formatting improves readability, the extensive style changes throughout the file may complicate review. Based on past feedback, the maintainer prefers to handle formatting separately.
🧹 Nitpick comments (1)
src/_terminal-utils.ts (1)
66-102: Solid buffering implementation for atomic terminal updates.The buffering mechanism is well-designed with proper state management and conditional sync mode wrapping. This will enable the atomic updates needed to reduce flickering.
However, consider improving the TTY check safety:
- if (this.syncMode && this.stream.isTTY) { + if (this.syncMode && this.stream?.isTTY) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 84f955bf56a4c32174d2ba2852f39dfc17f90aa7 and 5a1fb4062f90e0b69598d7c484a19b99c32d527a.
📒 Files selected for processing (3)
src/_consts.ts(1 hunks)src/_terminal-utils.ts(4 hunks)src/commands/_blocks.live.ts(17 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/_consts.ts
🧰 Additional context used
🪛 ESLint
src/_terminal-utils.ts
[error] 73-73: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 73-73: Unsafe member access .write on an error typed value.
(ts/no-unsafe-member-access)
[error] 93-93: Unexpected any value in conditional. An explicit comparison or type conversion is required.
(ts/strict-boolean-expressions)
[error] 93-93: Unsafe member access .isTTY on an error typed value.
(ts/no-unsafe-member-access)
[error] 94-94: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 94-94: Unsafe member access .write on an error typed value.
(ts/no-unsafe-member-access)
[error] 97-97: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 97-97: Unsafe member access .write on an error typed value.
(ts/no-unsafe-member-access)
[error] 109-109: Unexpected any value in conditional. An explicit comparison or type conversion is required.
(ts/strict-boolean-expressions)
[error] 109-109: Unsafe member access .isTTY on an error typed value.
(ts/no-unsafe-member-access)
[error] 110-110: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 110-110: Unsafe member access .write on an error typed value.
(ts/no-unsafe-member-access)
[error] 110-110: Unsafe member access .enterAlternativeScreen on an error typed value.
(ts/no-unsafe-member-access)
[error] 112-112: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 112-112: Unsafe member access .write on an error typed value.
(ts/no-unsafe-member-access)
[error] 122-122: Unexpected any value in conditional. An explicit comparison or type conversion is required.
(ts/strict-boolean-expressions)
[error] 122-122: Unsafe member access .isTTY on an error typed value.
(ts/no-unsafe-member-access)
[error] 124-124: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 124-124: Unsafe member access .write on an error typed value.
(ts/no-unsafe-member-access)
[error] 125-125: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 125-125: Unsafe member access .write on an error typed value.
(ts/no-unsafe-member-access)
[error] 125-125: Unsafe member access .exitAlternativeScreen on an error typed value.
(ts/no-unsafe-member-access)
src/commands/_blocks.live.ts
[error] 59-59: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 59-59: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 101-101: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 101-101: Unsafe construction of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 145-145: Unsafe member access .signal on an error typed value.
(ts/no-unsafe-member-access)
[error] 164-164: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 164-164: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 164-164: Unsafe member access .cursorTo on an error typed value.
(ts/no-unsafe-member-access)
[error] 165-165: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 165-165: Unsafe member access .eraseDown on an error typed value.
(ts/no-unsafe-member-access)
[error] 167-167: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 167-167: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 167-167: Unsafe member access .yellow on an error typed value.
(ts/no-unsafe-member-access)
[error] 170-170: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 170-170: Unsafe member access .cursorHide on an error typed value.
(ts/no-unsafe-member-access)
[error] 173-173: Unsafe member access .signal on an error typed value.
(ts/no-unsafe-member-access)
[error] 190-190: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 190-190: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 190-190: Unsafe member access .cursorTo on an error typed value.
(ts/no-unsafe-member-access)
[error] 191-191: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 191-191: Unsafe member access .eraseDown on an error typed value.
(ts/no-unsafe-member-access)
[error] 194-194: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 194-194: Unsafe member access .cursorHide on an error typed value.
(ts/no-unsafe-member-access)
[error] 200-200: Unsafe member access .signal on an error typed value.
(ts/no-unsafe-member-access)
[error] 225-225: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 225-225: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 225-225: Unsafe member access .red on an error typed value.
(ts/no-unsafe-member-access)
[error] 227-227: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 227-227: Unsafe member access .error on an error typed value.
(ts/no-unsafe-member-access)
[error] 229-229: Unsafe member access .signal on an error typed value.
(ts/no-unsafe-member-access)
[error] 309-309: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 309-309: Unsafe member access .bold on an error typed value.
(ts/no-unsafe-member-access)
[error] 315-315: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 315-315: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 315-315: Unsafe member access .bold on an error typed value.
(ts/no-unsafe-member-access)
[error] 316-316: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 316-316: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 320-320: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 340-340: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 366-366: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 366-366: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 367-367: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 367-367: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 367-367: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 371-371: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 371-371: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 371-371: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 371-371: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 375-382: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 378-378: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 378-378: Unsafe member access .red on an error typed value.
(ts/no-unsafe-member-access)
[error] 380-380: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 380-380: Unsafe member access .yellow on an error typed value.
(ts/no-unsafe-member-access)
[error] 381-381: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 381-381: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 385-385: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 385-385: Unsafe member access .bold on an error typed value.
(ts/no-unsafe-member-access)
[error] 386-386: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 386-386: Unsafe member access .bold on an error typed value.
(ts/no-unsafe-member-access)
[error] 389-389: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 389-389: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 389-389: Unsafe member access .bold on an error typed value.
(ts/no-unsafe-member-access)
[error] 390-390: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 390-390: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 395-395: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 415-415: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 422-422: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 438-438: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 472-472: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 472-472: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 473-473: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 473-473: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 473-473: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 478-478: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 478-478: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 478-478: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 478-478: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 480-487: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 483-483: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 483-483: Unsafe member access .red on an error typed value.
(ts/no-unsafe-member-access)
[error] 485-485: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 485-485: Unsafe member access .yellow on an error typed value.
(ts/no-unsafe-member-access)
[error] 486-486: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 486-486: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 487-487: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 487-487: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 490-490: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 490-490: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 490-490: Unsafe member access .bold on an error typed value.
(ts/no-unsafe-member-access)
[error] 491-491: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 491-491: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 496-496: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 517-517: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 525-525: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 546-546: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 560-560: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 568-568: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 568-568: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 599-604: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 601-601: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 601-601: Unsafe member access .red on an error typed value.
(ts/no-unsafe-member-access)
[error] 603-603: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 603-603: Unsafe member access .yellow on an error typed value.
(ts/no-unsafe-member-access)
[error] 604-604: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 604-604: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 625-625: Unsafe argument of type error typed assigned to a parameter of type string.
(ts/no-unsafe-argument)
[error] 625-625: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 625-625: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
🔇 Additional comments (10)
src/commands/_blocks.live.ts (6)
20-36: Good integration of new dependencies for enhanced terminal control.The new imports for
ansiEscapesandMIN_RENDER_INTERVAL_MSproperly support the frame rate limiting and cursor positioning features.
51-72: Well-implemented helper function for consistent abort handling.The
delayWithAbortfunction properly handles AbortSignal errors while preserving other error types. This will improve consistency across the monitoring loop.
139-157: Excellent frame rate limiting implementation.The logic correctly caps rendering at ~60 FPS using
MIN_RENDER_INTERVAL_MS, which will reduce CPU usage as intended. The abort signal handling is consistent with the new helper function.
119-127: Optimal terminal setup for reduced flickering.The sequence of entering alternate screen, enabling sync mode, clearing, and hiding cursor follows best practices for smooth terminal UI updates.
162-171: Efficient rendering with buffered output and cursor control.The use of cursor positioning, buffering, and explicit cursor hiding will significantly reduce flickering. The atomic flush operations ensure smooth updates.
Also applies to: 187-196
221-230: Improved error handling with consistent buffering.The error handling properly uses buffering and gracefully ignores abort errors during error recovery, ensuring clean error display even during shutdown.
src/_terminal-utils.ts (4)
6-10: Well-documented escape sequences with proper reference.The DEC mode 2026 escape sequences are properly documented with a reference link, addressing the previous feedback about documenting escape sequence sources.
104-128: Proper alternate screen buffer management.The implementation correctly manages the alternate screen state with appropriate TTY checking and line wrap control to prevent visual artifacts.
130-143: Clean sync mode management for atomic updates.The enable/disable sync mode methods are simple and effective, properly integrated with the flush method for conditional atomic update support.
175-176: Enhanced cleanup ensures complete terminal state restoration.The addition of alternate screen exit and sync mode disable to cleanup ensures the terminal is properly restored to normal state on program exit.
648a30f to
29d7a25
Compare
29d7a25 to
eaf5617
Compare
ryoppippi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@bobisme |
I noticed when I wasn't focused on the terminal in Ghostty, the live terminal had some bad flickering. The cursor was also still being shown.
NOTE: only tested in Ghostty
NOTE: couldn't figure out how to auto-format, eslint wasn't doing it, so I used prettier which may have caused too many changes
Summary by CodeRabbit
New Features
Bug Fixes
Style