-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
base: master
Are you sure you want to change the base?
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_added
color from bold red to bold green for better visual distinction - Unifies all diff comparisons to use
text_diff_added
andtext_diff_removed
colors 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.49% 66.53% +0.04%
==========================================
Files 117 117
Lines 18122 18099 -23
Branches 3071 3062 -9
==========================================
- Hits 12051 12043 -8
+ Misses 5415 5404 -11
+ Partials 656 652 -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
- 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`.
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.
text_diff_added
color from bold red to bold green; unify all diff/case comparisons to usetext_diff_added
andtext_diff_removed
colors consistently.ui
configuration docs.beet write -p year:2000..2005
Before
After
beet move -p albumtype:broadcast
Before
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: