-
Notifications
You must be signed in to change notification settings - Fork 2k
Improve default pretty diff colors #5949
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the default pretty diff colors in the beets music library tool by changing the default text_diff_added color from bold red to bold green and unifying diff/case comparisons to consistently use the new color scheme. Additionally, it simplifies color handling with a cached configuration system, consolidates regex patterns, removes unused color definitions, and updates documentation.
- Changes default
text_diff_addedcolor from bold red to bold green for better visual distinction - Unifies all diff comparisons to use
text_diff_addedandtext_diff_removedcolors consistently - Refactors color system with cached configuration validation and consolidated regex patterns
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/reference/config.rst | Updates configuration documentation with new color defaults and improved formatting |
| docs/changelog.rst | Documents the UI color changes in the changelog |
| beets/ui/commands.py | Refactors diff display logic to use cached prefix and consistent color naming |
| beets/ui/init.py | Major refactor of color system with cached config, validation, and simplified diff logic |
| beets/plugins.py | Changes plugin loading log level from info to debug |
| beets/config_default.yaml | Updates default color configuration removing unused colors and changing diff colors |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
094bc7f to
30e9aa0
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5949 +/- ##
==========================================
+ Coverage 66.48% 66.52% +0.04%
==========================================
Files 117 117
Lines 18107 18084 -23
Branches 3070 3061 -9
==========================================
- Hits 12038 12030 -8
+ Misses 5414 5403 -11
+ Partials 655 651 -4
🚀 New features to boost your workflow:
|
|
One idea for a small ergonomics improvement: right now the color setting requires an explicit This would mirror Django’s approach in django.core.management.color.supports_color. Would you be interested in adding this as part of the PR? I feel like it could fit. No pressure if not. Independent of that, I’ll take a closer look this weekend. I haven’t dug too deep into the CLI layer yet, so it might be a good spot for others to weigh in as well. |
|
There are a few related issues (for example, #2152 and #5837) - I'll be happy to fix them and implement your suggestion in a separate PR. Would you mind creating a small issue for this? I feel like your suggestion could be extended with a standard I'll start this right away, actually! |
30e9aa0 to
c78d97c
Compare
c78d97c to
7bb164f
Compare
7bb164f to
91a97ec
Compare
|
@sourcery-ai review |
Reviewer's GuideThis PR overhauls the pretty diff coloring by switching the default added color to green, refactoring and caching the color configuration with consolidated ANSI regexes, simplifying the diff and highlighting logic, and cleaning up unused color names alongside documentation updates. Entity relationship diagram for updated color configurationerDiagram
CONFIG ||--o{ COLOR_CONFIG : contains
COLOR_CONFIG {
color_name string
ansi_code string
}
COLOR_CONFIG ||--|{ ColorName : uses
ColorName {
name string
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
This is incredible! I was just thinking of doing the same but its already done! Awesome news. |
91a97ec to
a0e06bc
Compare
- Update default `text_diff_added` value: red -> green - Use `text_diff_removed` and `text_diff_added` instead of `text_error` in UI
This replaces the funky color setup based on a global `COLORS` variable with a cached function `get_color_config`.
a0e06bc to
30093c5
Compare
text_diff_addedcolor from bold red to bold green; unify all diff/case comparisons to usetext_diff_addedandtext_diff_removedcolors consistently.uiconfiguration docs.beet write -p year:2000..2005Before
After
beet move -p albumtype:broadcastBefore
After
Summary by Sourcery
Improve default diff colors from red to green and streamline color handling by refactoring ANSI code management, removing legacy logic, and unifying diff highlighting. Also extract a cached changed_prefix property and update UI config documentation.
Enhancements:
Documentation:
Chores: