🎨 Palette: Enhance CLI Summary Table and Progress Bar#326
Conversation
- Replaced the deprecated "light shade" character in the progress bar with a cleaner "middle dot" (`·`) character for reduced visual noise. - Implemented a new `print_summary_table` function to display the sync results in a structured table format. - Added support for Unicode box-drawing characters when colors are enabled, falling back to ASCII for compatibility. - Fixed header alignment logic to ensure correct padding before applying ANSI color codes. - Updated `tests/test_ux.py` to reflect the progress bar character change. 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. |
|
😎 Merged manually by @abhimehro - details. |
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
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 improves the command-line interface's visual presentation by introducing a more professional and structured summary table. It also refines the progress bar's appearance for a cleaner user experience. These changes enhance the overall clarity and aesthetic appeal of the CLI output, making it easier for users to digest information. 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
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| # Check for progress bar chars | ||
| assert "░" in combined_output | ||
| assert "·" in combined_output |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
![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 significantly enhances the CLI's visual output by refactoring the progress bar and introducing a well-formatted summary table. The move to a dedicated print_summary_table function is a great improvement for code organization. My review focuses on opportunities within this new function to further improve maintainability by reducing code duplication. The changes to the progress bar and the corresponding test updates look good.
| total_width = ( | ||
| 1 + (col_widths["profile"] + 2) + 1 + | ||
| (col_widths["folders"] + 2) + 1 + | ||
| (col_widths["rules"] + 2) + 1 + | ||
| (col_widths["duration"] + 2) + 1 + | ||
| (col_widths["status"] + 2) + 1 | ||
| ) |
There was a problem hiding this comment.
The calculation for total_width is repetitive and can be simplified by deriving it from the col_widths dictionary. This makes the code more maintainable, as it will automatically adjust if columns are added or removed.
total_width = sum(col_widths.values()) + (len(col_widths) * 3) + 1| for res in results: | ||
| # Profile | ||
| p_val = f"{res['profile']:<{col_widths['profile']}}" | ||
|
|
||
| # Folders | ||
| f_val = f"{res['folders']:>{col_widths['folders']}}" | ||
|
|
||
| # Rules | ||
| r_val = f"{res['rules']:>{col_widths['rules']},}" | ||
|
|
||
| # Duration | ||
| d_val = f"{res['duration']:>{col_widths['duration']-1}.1f}s" | ||
|
|
||
| # Status | ||
| status_label = res["status_label"] | ||
| s_val_raw = f"{status_label:<{col_widths['status']}}" | ||
| if USE_COLORS: | ||
| status_color = Colors.GREEN if res["success"] else Colors.FAIL | ||
| s_val = f"{status_color}{s_val_raw}{Colors.ENDC}" | ||
| else: | ||
| s_val = s_val_raw | ||
|
|
||
| print( | ||
| f"{chars['v']} {p_val} " | ||
| f"{chars['v']} {f_val} " | ||
| f"{chars['v']} {r_val} " | ||
| f"{chars['v']} {d_val} " | ||
| f"{chars['v']} {s_val} {chars['v']}" | ||
| ) | ||
|
|
||
| total_folders += res["folders"] | ||
| total_rules += res["rules"] | ||
| total_duration += res["duration"] | ||
| if res["success"]: | ||
| success_count += 1 | ||
|
|
||
| _print_separator("ml", "mm", "mr") | ||
|
|
||
| # Total Row | ||
| total = len(results) | ||
| all_success = success_count == total | ||
|
|
||
| if dry_run: | ||
| total_status_text = "✅ Ready" if all_success else "❌ Errors" | ||
| else: | ||
| total_status_text = "✅ All Good" if all_success else "❌ Errors" | ||
|
|
||
| p_val = f"{'TOTAL':<{col_widths['profile']}}" | ||
| if USE_COLORS: | ||
| p_val = f"{Colors.BOLD}{p_val}{Colors.ENDC}" | ||
|
|
||
| f_val = f"{total_folders:>{col_widths['folders']}}" | ||
| r_val = f"{total_rules:>{col_widths['rules']},}" | ||
| d_val = f"{total_duration:>{col_widths['duration']-1}.1f}s" | ||
|
|
||
| s_val_raw = f"{total_status_text:<{col_widths['status']}}" | ||
| if USE_COLORS: | ||
| status_color = Colors.GREEN if all_success else Colors.FAIL | ||
| s_val = f"{status_color}{s_val_raw}{Colors.ENDC}" | ||
| else: | ||
| s_val = s_val_raw | ||
|
|
||
| print( | ||
| f"{chars['v']} {p_val} " | ||
| f"{chars['v']} {f_val} " | ||
| f"{chars['v']} {r_val} " | ||
| f"{chars['v']} {d_val} " | ||
| f"{chars['v']} {s_val} {chars['v']}" | ||
| ) |
There was a problem hiding this comment.
There's significant code duplication in how data rows and the total row are formatted and printed. The logic within the for res in results: loop is nearly identical to the logic for printing the 'TOTAL' row.
To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, this shared logic should be extracted into a dedicated helper function. This function could handle the formatting, coloring, and printing of a data row, and be called from both the loop and for the total row. The existing _print_row function is only used for the header and is not suitable for data rows due to different formatting requirements.
There was a problem hiding this comment.
Pull request overview
This PR enhances the CLI's visual presentation by replacing the progress bar's light shade character (░) with a cleaner middle dot (·) and refactoring the summary table to use Unicode box-drawing characters (with ASCII fallback). The changes improve readability and provide a more professional appearance for the application's output.
Changes:
- Replaced progress bar unfilled character from
░to·in bothcountdown_timerandrender_progress_barfunctions - Extracted summary table logic from
main()into a newprint_summary_table()function with structured box-drawing characters - Updated test expectations to verify the new progress bar character
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| main.py | Added print_summary_table() function with Unicode/ASCII box-drawing support; updated progress bar characters in countdown_timer and render_progress_bar; removed inline table printing logic from main() |
| tests/test_ux.py | Updated test assertion to check for new middle dot character instead of light shade in progress bar output |
| def print_summary_table(results: List[Dict[str, Any]], dry_run: bool) -> None: | ||
| """Prints a nicely formatted summary table.""" | ||
| # Determine the width for the Profile ID column (min 25) | ||
| max_profile_len = max((len(r["profile"]) for r in results), default=25) | ||
| profile_col_width = max(25, max_profile_len) | ||
|
|
||
| # Calculate widths | ||
| col_widths = { | ||
| "profile": profile_col_width, | ||
| "folders": 10, | ||
| "rules": 10, | ||
| "duration": 10, | ||
| "status": 15, | ||
| } | ||
|
|
||
| if USE_COLORS: | ||
| # Unicode Box Drawing | ||
| chars = { | ||
| "tl": "┌", "tm": "┬", "tr": "┐", | ||
| "bl": "└", "bm": "┴", "br": "┘", | ||
| "ml": "├", "mm": "┼", "mr": "┤", | ||
| "v": "│", "h": "─", | ||
| } | ||
| else: | ||
| # ASCII Fallback | ||
| chars = { | ||
| "tl": "+", "tm": "+", "tr": "+", | ||
| "bl": "+", "bm": "+", "br": "+", | ||
| "ml": "+", "mm": "+", "mr": "+", | ||
| "v": "|", "h": "-", | ||
| } | ||
|
|
||
| def _print_separator(left, mid, right): | ||
| parts = [] | ||
| parts.append(chars[left]) | ||
| parts.append(chars["h"] * (col_widths["profile"] + 2)) | ||
| parts.append(chars[mid]) | ||
| parts.append(chars["h"] * (col_widths["folders"] + 2)) | ||
| parts.append(chars[mid]) | ||
| parts.append(chars["h"] * (col_widths["rules"] + 2)) | ||
| parts.append(chars[mid]) | ||
| parts.append(chars["h"] * (col_widths["duration"] + 2)) | ||
| parts.append(chars[mid]) | ||
| parts.append(chars["h"] * (col_widths["status"] + 2)) | ||
| parts.append(chars[right]) | ||
| print("".join(parts)) | ||
|
|
||
| def _print_row(profile, folders, rules, duration, status, is_header=False): | ||
| v = chars["v"] | ||
|
|
||
| # 1. Pad raw strings first (so padding is calculated on visible chars) | ||
| p_val = f"{profile:<{col_widths['profile']}}" | ||
| f_val = f"{folders:>{col_widths['folders']}}" | ||
| r_val = f"{rules:>{col_widths['rules']}}" | ||
| d_val = f"{duration:>{col_widths['duration']}}" | ||
| s_val = f"{status:<{col_widths['status']}}" | ||
|
|
||
| # 2. Wrap in color codes if needed | ||
| if is_header and USE_COLORS: | ||
| p_val = f"{Colors.BOLD}{p_val}{Colors.ENDC}" | ||
| f_val = f"{Colors.BOLD}{f_val}{Colors.ENDC}" | ||
| r_val = f"{Colors.BOLD}{r_val}{Colors.ENDC}" | ||
| d_val = f"{Colors.BOLD}{d_val}{Colors.ENDC}" | ||
| s_val = f"{Colors.BOLD}{s_val}{Colors.ENDC}" | ||
|
|
||
| print( | ||
| f"{v} {p_val} {v} {f_val} {v} {r_val} {v} {d_val} {v} {s_val} {v}" | ||
| ) | ||
|
|
||
| title_text = "DRY RUN SUMMARY" if dry_run else "SYNC SUMMARY" | ||
| title_color = Colors.CYAN if dry_run else Colors.HEADER | ||
|
|
||
| total_width = ( | ||
| 1 + (col_widths["profile"] + 2) + 1 + | ||
| (col_widths["folders"] + 2) + 1 + | ||
| (col_widths["rules"] + 2) + 1 + | ||
| (col_widths["duration"] + 2) + 1 + | ||
| (col_widths["status"] + 2) + 1 | ||
| ) | ||
|
|
||
| print("\n" + (f"{title_color}{title_text:^{total_width}}{Colors.ENDC}" if USE_COLORS else f"{title_text:^{total_width}}")) | ||
|
|
||
| _print_separator("tl", "tm", "tr") | ||
| # Header row - pad manually then print | ||
| _print_row("Profile ID", "Folders", "Rules", "Duration", "Status", is_header=True) | ||
| _print_separator("ml", "mm", "mr") | ||
|
|
||
| total_folders = 0 | ||
| total_rules = 0 | ||
| total_duration = 0.0 | ||
| success_count = 0 | ||
|
|
||
| for res in results: | ||
| # Profile | ||
| p_val = f"{res['profile']:<{col_widths['profile']}}" | ||
|
|
||
| # Folders | ||
| f_val = f"{res['folders']:>{col_widths['folders']}}" | ||
|
|
||
| # Rules | ||
| r_val = f"{res['rules']:>{col_widths['rules']},}" | ||
|
|
||
| # Duration | ||
| d_val = f"{res['duration']:>{col_widths['duration']-1}.1f}s" | ||
|
|
||
| # Status | ||
| status_label = res["status_label"] | ||
| s_val_raw = f"{status_label:<{col_widths['status']}}" | ||
| if USE_COLORS: | ||
| status_color = Colors.GREEN if res["success"] else Colors.FAIL | ||
| s_val = f"{status_color}{s_val_raw}{Colors.ENDC}" | ||
| else: | ||
| s_val = s_val_raw | ||
|
|
||
| print( | ||
| f"{chars['v']} {p_val} " | ||
| f"{chars['v']} {f_val} " | ||
| f"{chars['v']} {r_val} " | ||
| f"{chars['v']} {d_val} " | ||
| f"{chars['v']} {s_val} {chars['v']}" | ||
| ) | ||
|
|
||
| total_folders += res["folders"] | ||
| total_rules += res["rules"] | ||
| total_duration += res["duration"] | ||
| if res["success"]: | ||
| success_count += 1 | ||
|
|
||
| _print_separator("ml", "mm", "mr") | ||
|
|
||
| # Total Row | ||
| total = len(results) | ||
| all_success = success_count == total | ||
|
|
||
| if dry_run: | ||
| total_status_text = "✅ Ready" if all_success else "❌ Errors" | ||
| else: | ||
| total_status_text = "✅ All Good" if all_success else "❌ Errors" | ||
|
|
||
| p_val = f"{'TOTAL':<{col_widths['profile']}}" | ||
| if USE_COLORS: | ||
| p_val = f"{Colors.BOLD}{p_val}{Colors.ENDC}" | ||
|
|
||
| f_val = f"{total_folders:>{col_widths['folders']}}" | ||
| r_val = f"{total_rules:>{col_widths['rules']},}" | ||
| d_val = f"{total_duration:>{col_widths['duration']-1}.1f}s" | ||
|
|
||
| s_val_raw = f"{total_status_text:<{col_widths['status']}}" | ||
| if USE_COLORS: | ||
| status_color = Colors.GREEN if all_success else Colors.FAIL | ||
| s_val = f"{status_color}{s_val_raw}{Colors.ENDC}" | ||
| else: | ||
| s_val = s_val_raw | ||
|
|
||
| print( | ||
| f"{chars['v']} {p_val} " | ||
| f"{chars['v']} {f_val} " | ||
| f"{chars['v']} {r_val} " | ||
| f"{chars['v']} {d_val} " | ||
| f"{chars['v']} {s_val} {chars['v']}" | ||
| ) | ||
|
|
||
| _print_separator("bl", "bm", "br") |
There was a problem hiding this comment.
The new print_summary_table function lacks test coverage despite the codebase having comprehensive test coverage for similar display functions (see test_plan_details.py for examples). Tests should verify:
- Correct table formatting with and without colors
- Proper handling of Unicode vs ASCII box-drawing characters based on USE_COLORS
- Accurate calculation and display of totals
- Correct status indicators for dry-run vs real sync modes
- Edge cases like empty results list or very long profile names
| p_val = f"{'TOTAL':<{col_widths['profile']}}" | ||
| if USE_COLORS: | ||
| p_val = f"{Colors.BOLD}{p_val}{Colors.ENDC}" | ||
|
|
||
| f_val = f"{total_folders:>{col_widths['folders']}}" | ||
| r_val = f"{total_rules:>{col_widths['rules']},}" | ||
| d_val = f"{total_duration:>{col_widths['duration']-1}.1f}s" | ||
|
|
||
| s_val_raw = f"{total_status_text:<{col_widths['status']}}" | ||
| if USE_COLORS: | ||
| status_color = Colors.GREEN if all_success else Colors.FAIL | ||
| s_val = f"{status_color}{s_val_raw}{Colors.ENDC}" | ||
| else: | ||
| s_val = s_val_raw |
There was a problem hiding this comment.
The TOTAL row formatting differs from the old implementation. In the old code (removed lines), the entire TOTAL row was wrapped in bold (except the status which had its own color). In the new implementation, only the "TOTAL" text in the profile column is bolded (lines 437-439), while the numeric values (folders, rules, duration) are not bolded.
This changes the visual appearance of the summary table's total row. Consider wrapping all columns except status in bold to match the previous behavior, or update lines 441-443 to also be wrapped in bold when USE_COLORS is True.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
This PR enhances the CLI's visual presentation by:
░character with·(middle dot).tests/test_ux.pyand verified all tests pass.PR created automatically by Jules for task 1798974858941889427 started by @abhimehro