🎨 Palette: Hide terminal cursor during CLI spinner animation#8
Conversation
💡 What: Hides the terminal cursor (`\x1b[?25l`) while the `Spinner` is active and restores it (`\x1b[?25h`) when the spinner finishes, fails, or goes out of scope (via the `Drop` trait). 🎯 Why: Prevents the terminal cursor from jumping around or rendering alongside the spinner frames during long-running tasks, creating a smoother and more polished CLI experience. The `Drop` implementation ensures safety so the cursor is not permanently hidden if the CLI crashes or is interrupted. 📸 Before/After: Visual noise during "🦀 Thinking..." is reduced. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Code Review
This pull request implements terminal cursor management for the CLI spinner, hiding the cursor during animation and ensuring its restoration via the Drop trait. Feedback includes removing internal session artifacts, fixing an unexpanded shell command in the documentation, and refining the cursor restoration logic to be terminal-aware, which prevents side effects in non-terminal environments and unit tests.
|
@copilot code review |
|
@claude code review |
There was a problem hiding this comment.
Pull request overview
Improves CLI UX by hiding the terminal cursor while the Spinner is active and restoring it when the spinner completes or is dropped, reducing visual noise during long-running operations.
Changes:
- Hide the cursor on first spinner tick and restore it on success/failure and via
Drop. - Remove some
rust/.claw/sessions/*.jsonlsession artifacts. - Add new
.port_sessions/*.jsonartifacts and a Jules note file.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/crates/rusty-claude-cli/src/render.rs | Adds cursor hide/show behavior to the Spinner, including a Drop restore path. |
| rust/.claw/sessions/session-1775386853666-0.jsonl | Removes a local session artifact. |
| rust/.claw/sessions/session-1775386852257-0.jsonl | Removes a local session artifact. |
| rust/.claw/sessions/session-1775386842352-0.jsonl | Removes a local session artifact. |
| rust/.claw/sessions/session-1775386832313-0.jsonl | Removes a local session artifact. |
| .port_sessions/d5662f5a103444a9984129c1c8b01597.json | Adds a local session artifact (should not be committed). |
| .port_sessions/59b491f8b3b2439a957e0c680cd193bc.json | Adds a local session artifact (should not be committed). |
| .port_sessions/200c7e63f6f04a57ac6211a90e4e35d0.json | Adds a local session artifact (should not be committed). |
| .port_sessions/108fa52a890542199d833cd93ed0429f.json | Adds a local session artifact (should not be committed). |
| .Jules/palette.md | Adds a note documenting the cursor-hiding UX approach. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Agent-Logs-Url: https://github.com/badMade/claw-code/sessions/56bca0b3-24cf-4c30-a1e4-19d7b17df2c5 Co-authored-by: badMade <106821302+badMade@users.noreply.github.com>
Addressed with commit e71eaab. I fixed an error-path bug in spinner cursor restoration so |
|
@codex Code Review |
1 similar comment
|
@codex Code Review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e71eaab600
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex code review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e71eaab600
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let mut out = io::stdout(); | ||
| let _ = execute!(out, Show); |
There was a problem hiding this comment.
Restore cursor using the spinner output writer
Spinner::drop always emits Show to io::stdout(), but tick hides the cursor on whichever writer the caller passed in. When a non-stdout writer is used (e.g., stderr or an in-memory writer in tests), the hide/show pair goes to different destinations, which can leave the terminal cursor hidden or inject unexpected ANSI into stdout.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@copilot, make changes based on the above suggestion.
Agent-Logs-Url: https://github.com/badMade/claw-code/sessions/e0dbbfc8-a13d-4979-912a-62bab47f0163 Co-authored-by: badMade <106821302+badMade@users.noreply.github.com>
Completed in commit 495a33d: removed tracked
Completed in commit 495a33d: removed tracked
Completed in commit 495a33d: removed tracked |
|
To use Codex here, create a Codex account and connect to github. |
Completed review: no additional code changes were needed beyond commit 495a33d, and spinner-related tests pass ( |
🎨 Palette: Hide terminal cursor during CLI spinner animation
💡 What: Hides the terminal cursor (
\x1b[?25l) while theSpinneris active and restores it (\x1b[?25h) when the spinner finishes, fails, or goes out of scope (via theDroptrait).🎯 Why: Prevents the terminal cursor from jumping around or rendering alongside the spinner frames during long-running tasks, creating a smoother and more polished CLI experience. The
Dropimplementation ensures safety so the cursor is not permanently hidden if the CLI crashes or is interrupted.📸 Before/After: Visual noise during "🦀 Thinking..." is reduced.
PR created automatically by Jules for task 8229289081376148807 started by @badMade