🎨 Palette: Consistently use _print_hint for UX hints#731
🎨 Palette: Consistently use _print_hint for UX hints#731
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-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| monkeypatch.setattr(main, "USE_COLORS", False) | ||
| main._print_hint("💡 Hint: Just a test") | ||
| captured = capsys.readouterr() | ||
| assert "\033[2m" not in captured.out |
| main._print_hint("💡 Hint: Just a test") | ||
| captured = capsys.readouterr() | ||
| assert "\033[2m" not in captured.out | ||
| assert "💡 Hint: Just a test" in captured.out |
![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 refactors the printing of hint messages by introducing a _print_hint helper function, which centralizes color handling and reduces manual branching. Additionally, it updates the test suite to verify the new helper and improves type safety in existing tests. A review comment suggests further simplifying the 'No folders to sync' message by removing an unnecessary if USE_COLORS check, aligning it with the logic used in the rest of the PR.
| if USE_COLORS: | ||
| print(f" {Colors.WARNING}⚠️ No folders to sync.{Colors.ENDC}") | ||
| print( | ||
| f" {Colors.DIM}💡 Hint: Add folder URLs using --folder-url or in your config.yaml{Colors.ENDC}" | ||
| ) | ||
| else: | ||
| print(" ⚠️ No folders to sync.") |
There was a problem hiding this comment.
This if USE_COLORS block can be simplified to a single print statement. Since the Colors class attributes default to empty strings when USE_COLORS is False, the colored string will gracefully degrade to plain text. This change improves maintainability and aligns with the PR's objective of removing manual color branching at call sites, as seen in other parts of this refactor (e.g., lines 2871, 2928).
print(f" {Colors.WARNING}⚠️ No folders to sync.{Colors.ENDC}")There was a problem hiding this comment.
Pull request overview
Refactors CLI “hint” output in main.py to consistently route through the _print_hint() helper so hint styling (dim vs plain) is centralized and emoji-preserving across color/no-color modes.
Changes:
- Replaced several inline
print(... Colors.DIM ...)hint call sites with_print_hint(...)inmain.py. - Added a dedicated test to validate
_print_hintbehavior underUSE_COLORS=True/False. - Updated an existing UX test to construct
PlanEntrywith the correct TypedDict shape.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
main.py |
Swaps scattered hint print() logic for _print_hint() to unify styling behavior. |
tests/test_ux.py |
Adds _print_hint regression coverage and adjusts plan-entry test data to match TypedDicts. |
| @@ -460,20 +460,46 @@ def validator(value): | |||
| assert f"\033[2m{main.EMPTY_INPUT_HINT}\033[0m" in captured.out | |||
| assert f"\033[2m{main.INVALID_INPUT_HINT}\033[0m" in captured.out | |||
|
|
|||
Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>
|
|
||
| main._print_hint("💡 Hint: Just a test") | ||
| captured = capsys.readouterr() | ||
| assert "\033[2m💡 Hint: Just a test\033[0m" in captured.out |
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.
…ry-pick from #731) Replaces bare print() calls with _print_hint() for: - No folders to sync hint - Cache clear hint - Missing Profile ID hint - Missing API Token hint
|
Closing: #730 merged first and caused a merge conflict in |
Understood. Acknowledging that this work was closed due to a conflict and the changes have been cherry-picked manually. Stopping work on this task. |
…ry-pick #731) LiveReview Pre-Commit Check: skipped
What: Refactored direct
printstatements inmain.pyto use the custom_print_hinthelper function. Also added a dedicated test intests/test_ux.pyto ensure_print_hintappropriately styles the text based on theUSE_COLORSsetting without losing semantic emojis.Why: The codebase has a custom
_print_hint(hint: str)function designed to handle coloring and fallback of hints securely, but it was not consistently used for many other hints such as those related to "No folders to sync", cache clearing, missing Profile ID and missing API Token. This change simplifies the logic (removesif USE_COLORS:branching at the call sites) and aligns with the UX best practices logged in.jules/palette.mdto ensure emojis are preserved as uncolored text for fallback modes while keeping the CLI pretty when allowed.Before/After: No functional changes. The output now consistently preserves emojis in
NO_COLORmode for several hints.Accessibility: Improved fallback text mode rendering by ensuring semantic emojis are not stripped out, maintaining context and scannability.
PR created automatically by Jules for task 9985298205871170489 started by @abhimehro