🎨 Palette: Fix CLI color fallback UX for generic input prompts#720
🎨 Palette: Fix CLI color fallback UX for generic input prompts#720
Conversation
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
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
github-actions
bot
added
documentation
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![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 pull request implements conditional color output for interactive prompts and error messages to support environments where ANSI colors are disabled. It also updates the documentation to reflect these changes. Feedback was provided regarding code duplication between input functions, the use of stdout instead of stderr for interactive messages, and a potential initialization order bug where the USE_COLORS flag is set before environment variables are loaded from .env files.
main.py
Outdated
| if USE_COLORS: | ||
| print(f"\n{Colors.WARNING}⚠️ Input cancelled.{Colors.ENDC}") | ||
| else: | ||
| print("\n⚠️ Input cancelled.") |
There was a problem hiding this comment.
The logic for handling input cancellation and displaying error messages is now duplicated between get_validated_input and get_password. This increases maintenance overhead and the risk of UI inconsistencies.\n\nAdditionally, there are two other issues with this implementation:\n1. Output Stream: These interactive error messages and hints are printed to stdout. It is standard practice for CLI tools to use stderr for interactive prompts and errors to avoid polluting the data stream if the output is redirected.\n2. Initialization Order: The USE_COLORS flag is determined at the module level (lines 177-186) before load_dotenv() is called in main(). This means that NO_COLOR=1 set in a .env file will be ignored, and colors will still be enabled if the terminal is a TTY.\n\nConsider refactoring this logic into a shared helper function that directs output to sys.stderr and ensuring USE_COLORS is correctly initialized after the environment is fully loaded.
| if USE_COLORS: | |
| print(f"\n{Colors.WARNING}⚠️ Input cancelled.{Colors.ENDC}") | |
| else: | |
| print("\n⚠️ Input cancelled.") | |
| if USE_COLORS: | |
| print(f"\n{Colors.WARNING}⚠️ Input cancelled.{Colors.ENDC}", file=sys.stderr) | |
| else: | |
| print("\n⚠️ Input cancelled.", file=sys.stderr) |
There was a problem hiding this comment.
Pull request overview
Refines CLI prompt UX by making color vs. no-color output explicit in the generic input helpers, ensuring semantic fallback text (including emojis) is preserved when NO_COLOR=1 is used.
Changes:
- Converted input hint constants to plain (non-ANSI) strings.
- Added explicit
if USE_COLORSbranches inget_validated_inputandget_passwordfor cancel/empty/invalid output formatting. - Documented the palette learning/action for semantic color fallback behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| main.py | Makes prompt error/hint output explicitly conditional on USE_COLORS, and keeps hint strings ANSI-free. |
| .jules/palette.md | Adds a palette entry documenting the learned rule for color fallback in input prompts. |
main.py
Outdated
| if not value: | ||
| print(f"{Colors.FAIL}❌ Value cannot be empty{Colors.ENDC}") | ||
| print(EMPTY_INPUT_HINT) | ||
| if USE_COLORS: | ||
| print(f"{Colors.FAIL}❌ Value cannot be empty{Colors.ENDC}") | ||
| print(f"{Colors.DIM}{EMPTY_INPUT_HINT}{Colors.ENDC}") | ||
| else: |
There was a problem hiding this comment.
The new USE_COLORS branching for empty/invalid input output isn’t covered by tests. Given the repo already has NO_COLOR/UX tests, add unit tests that patch input()/getpass.getpass, toggle USE_COLORS, and assert the exact stdout lines (including emoji retention and DIM wrapping when colors are enabled) for both get_validated_input and get_password.
main.py
Outdated
| if USE_COLORS: | ||
| print(f"\n{Colors.WARNING}⚠️ Input cancelled.{Colors.ENDC}") | ||
| else: | ||
| print("\n⚠️ Input cancelled.") | ||
| sys.exit(130) |
There was a problem hiding this comment.
The color/no-color printing logic is now duplicated across get_validated_input and get_password (cancel, empty, invalid branches). Consider extracting a small shared helper (e.g., for printing an error + optional DIM hint) to avoid the two functions drifting in behavior over time.
Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
💡 What: Refactored generic input hint printing to use explicit
if USE_COLORSconditions rather than embedded empty string constants.🎯 Why: Fixes a UX issue where fallback text (like semantic emojis) might be inadvertently lost or improperly formatted when
NO_COLOR=1is used.📸 Before/After: None (terminal output parity).
♿ Accessibility: Improved semantic clarity in text-only terminals.
PR created automatically by Jules for task 14339738001532003484 started by @abhimehro