Skip to content

Commit 2b4758d

Browse files
authored
Improve default pretty diff colors (#5949)
- Switch default `text_diff_added` color from bold **red** to bold **green**; unify all diff/case comparisons to use `text_diff_added` and `text_diff_removed` colors consistently. - Simplify color handling and setup: introduce cached color config (validation + legacy normalization), consolidate regexes - Remove unused colors names - Update `ui` configuration docs. ### `beet write -p year:2000..2005` #### Before <img width="1261" height="460" alt="image" src="https://github.com/user-attachments/assets/70207350-6d9a-48d8-a314-457ac07c5ad1" /> #### After <img width="1257" height="536" alt="image" src="https://github.com/user-attachments/assets/efcf3453-016a-490f-84ab-dedfb2aca97b" /> ### `beet move -p albumtype:broadcast` #### Before <img width="1103" height="505" alt="image" src="https://github.com/user-attachments/assets/9e76c5d6-d878-4535-9da3-a949e6c0830c" /> #### After <img width="1134" height="508" alt="image" src="https://github.com/user-attachments/assets/3dc03988-8011-4072-8840-6f0a0d12350f" /> ## 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: - Introduce cached get_color_config and consolidate ANSI escape regex patterns to simplify color configuration parsing - Refactor diff highlighting to consistently use text_diff_added and text_diff_removed and simplify _colordiff implementation - Add ChangeRepresentation.changed_prefix cached property for consistent ‘not equal’ prefix formatting Documentation: - Update UI configuration documentation to reflect new default colors and removed settings Chores: - Remove unused color names and legacy normalization code
2 parents 160d407 + 30093c5 commit 2b4758d

File tree

5 files changed

+107
-160
lines changed

5 files changed

+107
-160
lines changed

beets/config_default.yaml

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -127,19 +127,12 @@ ui:
127127
action_default: ['bold', 'cyan']
128128
action: ['bold', 'cyan']
129129
# New Colors
130-
text: ['normal']
131130
text_faint: ['faint']
132131
import_path: ['bold', 'blue']
133132
import_path_items: ['bold', 'blue']
134-
added: ['green']
135-
removed: ['red']
136133
changed: ['yellow']
137-
added_highlight: ['bold', 'green']
138-
removed_highlight: ['bold', 'red']
139-
changed_highlight: ['bold', 'yellow']
140-
text_diff_added: ['bold', 'red']
134+
text_diff_added: ['bold', 'green']
141135
text_diff_removed: ['bold', 'red']
142-
text_diff_changed: ['bold', 'red']
143136
action_description: ['white']
144137
import:
145138
indentation:

beets/ui/__init__.py

Lines changed: 68 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@
3030
import traceback
3131
import warnings
3232
from difflib import SequenceMatcher
33-
from typing import Any, Callable
33+
from functools import cache
34+
from itertools import chain
35+
from typing import Any, Callable, Literal
3436

3537
import confuse
3638

@@ -438,7 +440,7 @@ def input_select_objects(prompt, objs, rep, prompt_all=None):
438440
# ANSI terminal colorization code heavily inspired by pygments:
439441
# https://bitbucket.org/birkenfeld/pygments-main/src/default/pygments/console.py
440442
# (pygments is by Tim Hatch, Armin Ronacher, et al.)
441-
COLOR_ESCAPE = "\x1b["
443+
COLOR_ESCAPE = "\x1b"
442444
LEGACY_COLORS = {
443445
"black": ["black"],
444446
"darkred": ["red"],
@@ -463,7 +465,7 @@ def input_select_objects(prompt, objs, rep, prompt_all=None):
463465
"white": ["bold", "white"],
464466
}
465467
# All ANSI Colors.
466-
ANSI_CODES = {
468+
CODE_BY_COLOR = {
467469
# Styles.
468470
"normal": 0,
469471
"bold": 1,
@@ -494,11 +496,17 @@ def input_select_objects(prompt, objs, rep, prompt_all=None):
494496
"bg_cyan": 46,
495497
"bg_white": 47,
496498
}
497-
RESET_COLOR = f"{COLOR_ESCAPE}39;49;00m"
498-
499-
# These abstract COLOR_NAMES are lazily mapped on to the actual color in COLORS
500-
# as they are defined in the configuration files, see function: colorize
501-
COLOR_NAMES = [
499+
RESET_COLOR = f"{COLOR_ESCAPE}[39;49;00m"
500+
# Precompile common ANSI-escape regex patterns
501+
ANSI_CODE_REGEX = re.compile(rf"({COLOR_ESCAPE}\[[;0-9]*m)")
502+
ESC_TEXT_REGEX = re.compile(
503+
rf"""(?P<pretext>[^{COLOR_ESCAPE}]*)
504+
(?P<esc>(?:{ANSI_CODE_REGEX.pattern})+)
505+
(?P<text>[^{COLOR_ESCAPE}]+)(?P<reset>{re.escape(RESET_COLOR)})
506+
(?P<posttext>[^{COLOR_ESCAPE}]*)""",
507+
re.VERBOSE,
508+
)
509+
ColorName = Literal[
502510
"text_success",
503511
"text_warning",
504512
"text_error",
@@ -507,76 +515,54 @@ def input_select_objects(prompt, objs, rep, prompt_all=None):
507515
"action_default",
508516
"action",
509517
# New Colors
510-
"text",
511518
"text_faint",
512519
"import_path",
513520
"import_path_items",
514521
"action_description",
515-
"added",
516-
"removed",
517522
"changed",
518-
"added_highlight",
519-
"removed_highlight",
520-
"changed_highlight",
521523
"text_diff_added",
522524
"text_diff_removed",
523-
"text_diff_changed",
524525
]
525-
COLORS: dict[str, list[str]] | None = None
526526

527527

528-
def _colorize(color, text):
529-
"""Returns a string that prints the given text in the given color
530-
in a terminal that is ANSI color-aware. The color must be a list of strings
531-
from ANSI_CODES.
528+
@cache
529+
def get_color_config() -> dict[ColorName, str]:
530+
"""Parse and validate color configuration, converting names to ANSI codes.
531+
532+
Processes the UI color configuration, handling both new list format and
533+
legacy single-color format. Validates all color names against known codes
534+
and raises an error for any invalid entries.
532535
"""
533-
# Construct escape sequence to be put before the text by iterating
534-
# over all "ANSI codes" in `color`.
535-
escape = ""
536-
for code in color:
537-
escape = f"{escape}{COLOR_ESCAPE}{ANSI_CODES[code]}m"
538-
return f"{escape}{text}{RESET_COLOR}"
536+
colors_by_color_name: dict[ColorName, list[str]] = {
537+
k: (v if isinstance(v, list) else LEGACY_COLORS.get(v, [v]))
538+
for k, v in config["ui"]["colors"].flatten().items()
539+
}
540+
541+
if invalid_colors := (
542+
set(chain.from_iterable(colors_by_color_name.values()))
543+
- CODE_BY_COLOR.keys()
544+
):
545+
raise UserError(
546+
f"Invalid color(s) in configuration: {', '.join(invalid_colors)}"
547+
)
539548

549+
return {
550+
n: ";".join(str(CODE_BY_COLOR[c]) for c in colors)
551+
for n, colors in colors_by_color_name.items()
552+
}
540553

541-
def colorize(color_name, text):
542-
"""Colorize text if colored output is enabled. (Like _colorize but
543-
conditional.)
554+
555+
def colorize(color_name: ColorName, text: str) -> str:
556+
"""Apply ANSI color formatting to text based on configuration settings.
557+
558+
Returns colored text when color output is enabled and NO_COLOR environment
559+
variable is not set, otherwise returns plain text unchanged.
544560
"""
545561
if config["ui"]["color"] and "NO_COLOR" not in os.environ:
546-
global COLORS
547-
if not COLORS:
548-
# Read all color configurations and set global variable COLORS.
549-
COLORS = dict()
550-
for name in COLOR_NAMES:
551-
# Convert legacy color definitions (strings) into the new
552-
# list-based color definitions. Do this by trying to read the
553-
# color definition from the configuration as unicode - if this
554-
# is successful, the color definition is a legacy definition
555-
# and has to be converted.
556-
try:
557-
color_def = config["ui"]["colors"][name].get(str)
558-
except (confuse.ConfigTypeError, NameError):
559-
# Normal color definition (type: list of unicode).
560-
color_def = config["ui"]["colors"][name].get(list)
561-
else:
562-
# Legacy color definition (type: unicode). Convert.
563-
if color_def in LEGACY_COLORS:
564-
color_def = LEGACY_COLORS[color_def]
565-
else:
566-
raise UserError("no such color %s", color_def)
567-
for code in color_def:
568-
if code not in ANSI_CODES.keys():
569-
raise ValueError("no such ANSI code %s", code)
570-
COLORS[name] = color_def
571-
# In case a 3rd party plugin is still passing the actual color ('red')
572-
# instead of the abstract color name ('text_error')
573-
color = COLORS.get(color_name)
574-
if not color:
575-
log.debug("Invalid color_name: {}", color_name)
576-
color = color_name
577-
return _colorize(color, text)
578-
else:
579-
return text
562+
color_code = get_color_config()[color_name]
563+
return f"{COLOR_ESCAPE}[{color_code}m{text}{RESET_COLOR}"
564+
565+
return text
580566

581567

582568
def uncolorize(colored_text):
@@ -589,26 +575,22 @@ def uncolorize(colored_text):
589575
# [;\d]* - matches a sequence consisting of one or more digits or
590576
# semicola
591577
# [A-Za-z] - matches a letter
592-
ansi_code_regex = re.compile(r"\x1b\[[;\d]*[A-Za-z]", re.VERBOSE)
593-
# Strip ANSI codes from `colored_text` using the regular expression.
594-
text = ansi_code_regex.sub("", colored_text)
595-
return text
578+
return ANSI_CODE_REGEX.sub("", colored_text)
596579

597580

598581
def color_split(colored_text, index):
599-
ansi_code_regex = re.compile(r"(\x1b\[[;\d]*[A-Za-z])", re.VERBOSE)
600582
length = 0
601583
pre_split = ""
602584
post_split = ""
603585
found_color_code = None
604586
found_split = False
605-
for part in ansi_code_regex.split(colored_text):
587+
for part in ANSI_CODE_REGEX.split(colored_text):
606588
# Count how many real letters we have passed
607589
length += color_len(part)
608590
if found_split:
609591
post_split += part
610592
else:
611-
if ansi_code_regex.match(part):
593+
if ANSI_CODE_REGEX.match(part):
612594
# This is a color code
613595
if part == RESET_COLOR:
614596
found_color_code = None
@@ -642,7 +624,7 @@ def color_len(colored_text):
642624
return len(uncolorize(colored_text))
643625

644626

645-
def _colordiff(a, b):
627+
def _colordiff(a: Any, b: Any) -> tuple[str, str]:
646628
"""Given two values, return the same pair of strings except with
647629
their differences highlighted in the specified color. Strings are
648630
highlighted intelligently to show differences; other values are
@@ -664,35 +646,21 @@ def _colordiff(a, b):
664646
colorize("text_diff_added", str(b)),
665647
)
666648

667-
a_out = []
668-
b_out = []
649+
before = ""
650+
after = ""
669651

670652
matcher = SequenceMatcher(lambda x: False, a, b)
671653
for op, a_start, a_end, b_start, b_end in matcher.get_opcodes():
672-
if op == "equal":
673-
# In both strings.
674-
a_out.append(a[a_start:a_end])
675-
b_out.append(b[b_start:b_end])
676-
elif op == "insert":
677-
# Right only.
678-
b_out.append(colorize("text_diff_added", b[b_start:b_end]))
679-
elif op == "delete":
680-
# Left only.
681-
a_out.append(colorize("text_diff_removed", a[a_start:a_end]))
682-
elif op == "replace":
683-
# Right and left differ. Colorise with second highlight if
684-
# it's just a case change.
685-
if a[a_start:a_end].lower() != b[b_start:b_end].lower():
686-
a_color = "text_diff_removed"
687-
b_color = "text_diff_added"
688-
else:
689-
a_color = b_color = "text_highlight_minor"
690-
a_out.append(colorize(a_color, a[a_start:a_end]))
691-
b_out.append(colorize(b_color, b[b_start:b_end]))
692-
else:
693-
assert False
654+
before_part, after_part = a[a_start:a_end], b[b_start:b_end]
655+
if op in {"delete", "replace"}:
656+
before_part = colorize("text_diff_removed", before_part)
657+
if op in {"insert", "replace"}:
658+
after_part = colorize("text_diff_added", after_part)
659+
660+
before += before_part
661+
after += after_part
694662

695-
return "".join(a_out), "".join(b_out)
663+
return before, after
696664

697665

698666
def colordiff(a, b):
@@ -765,19 +733,13 @@ def split_into_lines(string, width_tuple):
765733
"""
766734
first_width, middle_width, last_width = width_tuple
767735
words = []
768-
esc_text = re.compile(
769-
r"""(?P<pretext>[^\x1b]*)
770-
(?P<esc>(?:\x1b\[[;\d]*[A-Za-z])+)
771-
(?P<text>[^\x1b]+)(?P<reset>\x1b\[39;49;00m)
772-
(?P<posttext>[^\x1b]*)""",
773-
re.VERBOSE,
774-
)
736+
775737
if uncolorize(string) == string:
776738
# No colors in string
777739
words = string.split()
778740
else:
779741
# Use a regex to find escapes and the text within them.
780-
for m in esc_text.finditer(string):
742+
for m in ESC_TEXT_REGEX.finditer(string):
781743
# m contains four groups:
782744
# pretext - any text before escape sequence
783745
# esc - intitial escape sequence
@@ -1110,8 +1072,8 @@ def _field_diff(field, old, old_fmt, new, new_fmt):
11101072
if isinstance(oldval, str):
11111073
oldstr, newstr = colordiff(oldval, newstr)
11121074
else:
1113-
oldstr = colorize("text_error", oldstr)
1114-
newstr = colorize("text_error", newstr)
1075+
oldstr = colorize("text_diff_removed", oldstr)
1076+
newstr = colorize("text_diff_added", newstr)
11151077

11161078
return f"{oldstr} -> {newstr}"
11171079

beets/ui/commands.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import textwrap
2222
from collections import Counter
2323
from collections.abc import Sequence
24+
from functools import cached_property
2425
from itertools import chain
2526
from platform import python_version
2627
from typing import Any, NamedTuple
@@ -303,6 +304,10 @@ class ChangeRepresentation:
303304
TrackMatch object, accordingly.
304305
"""
305306

307+
@cached_property
308+
def changed_prefix(self) -> str:
309+
return ui.colorize("changed", "\u2260")
310+
306311
cur_artist = None
307312
# cur_album set if album, cur_title set if singleton
308313
cur_album = None
@@ -394,17 +399,15 @@ def show_match_details(self):
394399
"""Print out the details of the match, including changes in album name
395400
and artist name.
396401
"""
397-
changed_prefix = ui.colorize("changed", "\u2260")
398402
# Artist.
399403
artist_l, artist_r = self.cur_artist or "", self.match.info.artist
400404
if artist_r == VARIOUS_ARTISTS:
401405
# Hide artists for VA releases.
402406
artist_l, artist_r = "", ""
403407
if artist_l != artist_r:
404408
artist_l, artist_r = ui.colordiff(artist_l, artist_r)
405-
# Prefix with U+2260: Not Equal To
406409
left = {
407-
"prefix": f"{changed_prefix} Artist: ",
410+
"prefix": f"{self.changed_prefix} Artist: ",
408411
"contents": artist_l,
409412
"suffix": "",
410413
}
@@ -422,9 +425,8 @@ def show_match_details(self):
422425
and self.match.info.album != VARIOUS_ARTISTS
423426
):
424427
album_l, album_r = ui.colordiff(album_l, album_r)
425-
# Prefix with U+2260: Not Equal To
426428
left = {
427-
"prefix": f"{changed_prefix} Album: ",
429+
"prefix": f"{self.changed_prefix} Album: ",
428430
"contents": album_l,
429431
"suffix": "",
430432
}
@@ -437,9 +439,8 @@ def show_match_details(self):
437439
title_l, title_r = self.cur_title or "", self.match.info.title
438440
if self.cur_title != self.match.info.title:
439441
title_l, title_r = ui.colordiff(title_l, title_r)
440-
# Prefix with U+2260: Not Equal To
441442
left = {
442-
"prefix": f"{changed_prefix} Title: ",
443+
"prefix": f"{self.changed_prefix} Title: ",
443444
"contents": title_l,
444445
"suffix": "",
445446
}
@@ -568,9 +569,8 @@ def make_line(self, item, track_info):
568569
# the case, thus the 'info' dictionary is unneeded.
569570
# penalties = penalty_string(self.match.distance.tracks[track_info])
570571

571-
prefix = ui.colorize("changed", "\u2260 ") if changed else "* "
572572
lhs = {
573-
"prefix": f"{prefix}{lhs_track} ",
573+
"prefix": f"{self.changed_prefix if changed else '*'} {lhs_track} ",
574574
"contents": lhs_title,
575575
"suffix": f" {lhs_length}",
576576
}

docs/changelog.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,10 @@ Other changes:
135135
file. :bug:`5979`
136136
- :doc:`plugins/lastgenre`: Updated and streamlined the genre whitelist and
137137
canonicalization tree :bug:`5977`
138+
- UI: Update default ``text_diff_added`` color from **bold red** to **bold
139+
green.**
140+
- UI: Use ``text_diff_added`` and ``text_diff_removed`` colors in **all** diff
141+
comparisons, including case differences.
138142

139143
2.3.1 (May 14, 2025)
140144
--------------------

0 commit comments

Comments
 (0)