🎨 Palette: Polish CLI progress bar & fix cursor visibility#165
🎨 Palette: Polish CLI progress bar & fix cursor visibility#165
Conversation
- Hide cursor during progress bar updates to prevent flickering - Restore cursor on exit using `atexit` - Increase progress bar width for better readability - Remove duplicate `render_progress_bar` implementation Co-authored-by: abhimehro <84992105+abhimehro@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. |
|
Merging to
|
Summary of ChangesHello @abhimehro, 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 significantly enhances the command-line interface (CLI) user experience by refining the progress bar component. The changes focus on improving visual fluidity and robustness, ensuring that users encounter a professional and stable terminal output during operations. The primary impact is a more polished and less distracting interaction with the application's progress indicators. Highlights
Changelog
Activity
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is a great pull request that significantly improves the command-line user experience. The use of atexit to manage cursor visibility is a robust solution for ensuring the terminal is left in a clean state, even if errors occur. The code cleanup by removing the duplicated function and the visual enhancement of the progress bar are also welcome changes. The implementation is clear and effective.
| sys.stderr.write(Colors.CURSOR_SHOW) | ||
| sys.stderr.flush() |
There was a problem hiding this comment.
To make the atexit handler more robust, consider wrapping the write operation in a try...except block. In the edge case where sys.stderr is closed or otherwise unavailable when this handler runs, sys.stderr.write() would raise an exception, leading to a traceback on exit. This change will prevent that and ensure a cleaner exit.
| sys.stderr.write(Colors.CURSOR_SHOW) | |
| sys.stderr.flush() | |
| try: | |
| sys.stderr.write(Colors.CURSOR_SHOW) | |
| sys.stderr.flush() | |
| except Exception: | |
| # stderr might be closed, nothing we can do. | |
| pass |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| # Use \033[K to clear line residue | ||
| sys.stderr.write( | ||
| f"\r\033[K{Colors.CYAN}{prefix} {label}: [{bar}] {percent}% ({current}/{total}){Colors.ENDC}" | ||
| f"\r{Colors.CURSOR_HIDE}\033[K{Colors.CYAN}{prefix} {label}: [{bar}] {percent}% ({current}/{total}){Colors.ENDC}" |
Check warning
Code scanning / Pylint (reported by Codacy)
Line too long (121/100) Warning
| # Use \033[K to clear line residue | ||
| sys.stderr.write( | ||
| f"\r\033[K{Colors.CYAN}{prefix} {label}: [{bar}] {percent}% ({current}/{total}){Colors.ENDC}" | ||
| f"\r{Colors.CURSOR_HIDE}\033[K{Colors.CYAN}{prefix} {label}: [{bar}] {percent}% ({current}/{total}){Colors.ENDC}" |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Line too long (121/100) Warning
There was a problem hiding this comment.
Pull request overview
This PR enhances the CLI user experience by improving the progress bar component with better cursor management and visual polish. The changes introduce cursor hiding during progress updates to prevent flickering, ensure cursor restoration via atexit handlers even on errors, remove duplicate code, and increase the progress bar width for better granularity.
Changes:
- Added cursor hide/show ANSI escape sequences to the Colors class and implemented an atexit handler to restore cursor visibility on script exit
- Increased progress bar width from 15 to 30 characters and added cursor hiding to both render_progress_bar and countdown_timer functions
- Removed duplicate render_progress_bar function definition (lines 177-195 in the original code)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| main.py | Added atexit import, CURSOR_HIDE/CURSOR_SHOW constants to Colors class, restore_cursor function with atexit registration, removed duplicate render_progress_bar function, added cursor hiding to countdown_timer and render_progress_bar, increased progress bar width to 30 |
| .jules/palette.md | Added documentation about CLI cursor hygiene learnings and best practices |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| f"\r{Colors.CURSOR_HIDE}\033[K{Colors.CYAN}{prefix} {label}: [{bar}] {percent}% ({current}/{total}){Colors.ENDC}" | ||
| ) | ||
| sys.stderr.flush() | ||
|
|
There was a problem hiding this comment.
The render_progress_bar function now includes cursor hiding functionality (CURSOR_HIDE), but the existing test in test_main.py:491-512 doesn't verify this new escape sequence is present in the output. Consider updating the test to assert that the CURSOR_HIDE sequence is included when USE_COLORS is enabled.
| def restore_cursor(): | ||
| """Ensure cursor is visible when the script exits.""" | ||
| if USE_COLORS: | ||
| sys.stderr.write(Colors.CURSOR_SHOW) | ||
| sys.stderr.flush() | ||
|
|
||
|
|
||
| atexit.register(restore_cursor) |
There was a problem hiding this comment.
The new restore_cursor function and its atexit registration lack test coverage. Consider adding a test that verifies the function writes CURSOR_SHOW to stderr when USE_COLORS is True, and verifies the atexit registration. This is important for ensuring the cursor restoration mechanism works correctly.
| f"\r{Colors.CURSOR_HIDE}{Colors.CYAN}⏳ {message}: [{bar}] {remaining}s...{Colors.ENDC}" | ||
| ) | ||
| sys.stderr.flush() | ||
| time.sleep(1) |
There was a problem hiding this comment.
The countdown_timer function now includes cursor hiding (CURSOR_HIDE on line 189), but there's no test coverage for this function in test_main.py. Since render_progress_bar has test coverage, countdown_timer should also have tests to verify the cursor hiding behavior and the countdown functionality.
| f"\r{Colors.CURSOR_HIDE}{Colors.CYAN}⏳ {message}: [{bar}] {remaining}s...{Colors.ENDC}" | ||
| ) | ||
| sys.stderr.flush() | ||
| time.sleep(1) |
There was a problem hiding this comment.
The countdown_timer function hides the cursor on line 189 but doesn't explicitly restore it when it completes on lines 194-195. While the atexit handler will restore it eventually, it's better to restore the cursor immediately after the timer finishes to avoid leaving it hidden longer than necessary. Consider adding the cursor restoration code before this line.
…itization, add dry-run plan details Incorporates the best changes from 36 Jules PRs, addressing review feedback: Bolt (Performance) - from PR #173: - Pre-compile PROFILE_ID_PATTERN and RULE_PATTERN at module level - Use compiled patterns in is_valid_profile_id_format, validate_profile_id, and is_valid_rule - Supersedes PRs: #140, #143, #152, #155, #158, #161, #167, #170, #173 Sentinel (Security) - from PR #172 with review feedback: - Enhance sanitize_for_log to redact Basic Auth credentials in URLs - Redact sensitive query parameters (token, key, secret, password, etc.) - Handle fragment separators (#) per Gemini Code Assist review - Use [^&#\s]* pattern per Copilot reviewer suggestion - Update docstring per reviewer suggestion - Supersedes PRs: #142, #145, #148, #151, #154, #157, #160, #169, #172 Palette (UX) - from PR #174 with lint fixes: - Add print_plan_details function for dry-run visibility - Fix duplicate render_progress_bar definition bug - Supersedes PRs: #139, #141, #144, #147, #150, #153, #156, #159, #162, #165, #168, #171, #174 Also: #146, #149, #164 (parallel folder deletion) and #166 (auto-fix .env perms) are independent features not consolidated here. Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>
abhimehro
deleted the
palette-cli-progress-cursor-fix-913151079959627518
branch
…itization, add dry-run plan details Incorporates the best changes from 36 Jules PRs, addressing review feedback: Bolt (Performance) - from PR #173: - Pre-compile PROFILE_ID_PATTERN and RULE_PATTERN at module level - Use compiled patterns in is_valid_profile_id_format, validate_profile_id, and is_valid_rule - Supersedes PRs: #140, #143, #152, #155, #158, #161, #167, #170, #173 Sentinel (Security) - from PR #172 with review feedback: - Enhance sanitize_for_log to redact Basic Auth credentials in URLs - Redact sensitive query parameters (token, key, secret, password, etc.) - Handle fragment separators (#) per Gemini Code Assist review - Use [^&#\s]* pattern per Copilot reviewer suggestion - Update docstring per reviewer suggestion - Supersedes PRs: #142, #145, #148, #151, #154, #157, #160, #169, #172 Palette (UX) - from PR #174 with lint fixes: - Add print_plan_details function for dry-run visibility - Fix duplicate render_progress_bar definition bug - Supersedes PRs: #139, #141, #144, #147, #150, #153, #156, #159, #162, #165, #168, #171, #174 Also: #146, #149, #164 (parallel folder deletion) and #166 (auto-fix .env perms) are independent features not consolidated here. Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>
This PR improves the CLI user experience by polishing the progress bar component.
Changes:
\033[?25l) during progress updates and restored (\033[?25h) upon exit (even on errors). This prevents the annoying "flickering cursor" effect.render_progress_bar.Tested with
uv run python main.py --dry-runand verified cursor behavior and table output. Existing tests passed.PR created automatically by Jules for task 913151079959627518 started by @abhimehro