From be290e54441987fdb80437f7982f4d5ad01bac59 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Sat, 14 Oct 2023 10:42:48 +0200 Subject: [PATCH 01/14] A large code-overhaul of the beets ui: - Allow user to change UI colors in config file. - "Change Representation" class allows Albums and Track matches to reuse similar formatting code - Functions to split text into lines for printing - Tests for the new UI to check wrapping functions --- beets/config_default.yaml | 35 +- beets/ui/__init__.py | 527 +++++++++++++++++++++++++----- beets/ui/commands.py | 669 +++++++++++++++++++++++++------------- test/test_ui.py | 87 ++++- 4 files changed, 1000 insertions(+), 318 deletions(-) diff --git a/beets/config_default.yaml b/beets/config_default.yaml index 358614d00..114e9b6e8 100644 --- a/beets/config_default.yaml +++ b/beets/config_default.yaml @@ -97,13 +97,34 @@ ui: length_diff_thresh: 10.0 color: yes colors: - text_success: green - text_warning: yellow - text_error: red - text_highlight: red - text_highlight_minor: lightgray - action_default: turquoise - action: blue + text_success: ['bold', 'green'] + text_warning: ['bold', 'yellow'] + text_error: ['bold', 'red'] + text_highlight: ['bold', 'red'] + text_highlight_minor: ['white'] + action_default: ['bold', 'cyan'] + action: ['bold', 'blue'] + # New Colors + text: ['normal'] + text_faint: ['faint'] + import_path: ['bold', 'inverse', 'blue'] + import_path_items: ['bold', 'blue'] + added: ['green'] + removed: ['red'] + changed: ['yellow'] + added_highlight: ['bold', 'green'] + removed_highlight: ['bold', 'red'] + changed_highlight: ['bold', 'yellow'] + text_diff_added: ['bold', 'red'] + text_diff_removed: ['bold', 'red'] + text_diff_changed: ['bold', 'red'] + action_description: ['blue'] + import: + indentation: + match_header: 2 + match_details: 2 + match_tracklist: 5 + layout: column format_item: $artist - $album - $title format_album: $albumartist - $album diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index cd9f4989e..37131b203 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -184,6 +184,13 @@ def should_move(move_opt=None): # Input prompts. + +def indent(count): + """Returns a string with `count` many spaces. + """ + return " " * count + + def input_(prompt=None): """Like `input`, but decodes the result to a Unicode string. Raises a UserError if stdin is not available. The prompt is sent to @@ -267,8 +274,11 @@ def input_options(options, require=False, prompt=None, fallback_prompt=None, show_letter) # Insert the highlighted letter back into the word. + descr_color = "action_default" if is_default else "action_description" capitalized.append( - option[:index] + show_letter + option[index + 1:] + colorize(descr_color, option[:index]) + + show_letter + + colorize(descr_color, option[index + 1:]) ) display_letters.append(found_letter.upper()) @@ -301,15 +311,16 @@ def input_options(options, require=False, prompt=None, fallback_prompt=None, prompt_part_lengths += [len(s) for s in options] # Wrap the query text. - prompt = '' + # Start prompt with U+279C: Heavy Round-Tipped Rightwards Arrow + prompt = colorize("action", "\u279C ") line_length = 0 for i, (part, length) in enumerate(zip(prompt_parts, prompt_part_lengths)): # Add punctuation. if i == len(prompt_parts) - 1: - part += '?' + part += colorize("action_description", "?") else: - part += ',' + part += colorize("action_description", ",") length += 1 # Choose either the current line or the beginning of the next. @@ -368,10 +379,12 @@ def input_yn(prompt, require=False): """Prompts the user for a "yes" or "no" response. The default is "yes" unless `require` is `True`, in which case there is no default. """ - sel = input_options( - ('y', 'n'), require, prompt, 'Enter Y or N:' + # Start prompt with U+279C: Heavy Round-Tipped Rightwards Arrow + yesno = colorize("action", "\u279C ") + colorize( + "action_description", "Enter Y or N:" ) - return sel == 'y' + sel = input_options(("y", "n"), require, prompt, yesno) + return sel == "y" def input_select_objects(prompt, objs, rep, prompt_all=None): @@ -465,97 +478,200 @@ def human_seconds_short(interval): # https://bitbucket.org/birkenfeld/pygments-main/src/default/pygments/console.py # (pygments is by Tim Hatch, Armin Ronacher, et al.) COLOR_ESCAPE = "\x1b[" -DARK_COLORS = { - "black": 0, - "darkred": 1, - "darkgreen": 2, - "brown": 3, - "darkyellow": 3, - "darkblue": 4, - "purple": 5, - "darkmagenta": 5, - "teal": 6, - "darkcyan": 6, - "lightgray": 7 +LEGACY_COLORS = { + "black": ["black"], + "darkred": ["red"], + "darkgreen": ["green"], + "brown": ["yellow"], + "darkyellow": ["yellow"], + "darkblue": ["blue"], + "purple": ["magenta"], + "darkmagenta": ["magenta"], + "teal": ["cyan"], + "darkcyan": ["cyan"], + "lightgray": ["white"], + "darkgray": ["bold", "black"], + "red": ["bold", "red"], + "green": ["bold", "green"], + "yellow": ["bold", "yellow"], + "blue": ["bold", "blue"], + "fuchsia": ["bold", "magenta"], + "magenta": ["bold", "magenta"], + "turquoise": ["bold", "cyan"], + "cyan": ["bold", "cyan"], + "white": ["bold", "white"], } -LIGHT_COLORS = { - "darkgray": 0, - "red": 1, - "green": 2, - "yellow": 3, - "blue": 4, - "fuchsia": 5, - "magenta": 5, - "turquoise": 6, - "cyan": 6, - "white": 7 +# All ANSI Colors. +ANSI_CODES = { + # Styles. + "normal": 0, + "bold": 1, + "faint": 2, + # "italic": 3, + "underline": 4, + # "blink_slow": 5, + # "blink_rapid": 6, + "inverse": 7, + # "conceal": 8, + # "crossed_out": 9 + # Text colors. + "black": 30, + "red": 31, + "green": 32, + "yellow": 33, + "blue": 34, + "magenta": 35, + "cyan": 36, + "white": 37, + # Background colors. + "bg_black": 40, + "bg_red": 41, + "bg_green": 42, + "bg_yellow": 43, + "bg_blue": 44, + "bg_magenta": 45, + "bg_cyan": 46, + "bg_white": 47, } RESET_COLOR = COLOR_ESCAPE + "39;49;00m" # These abstract COLOR_NAMES are lazily mapped on to the actual color in COLORS # as they are defined in the configuration files, see function: colorize -COLOR_NAMES = ['text_success', 'text_warning', 'text_error', 'text_highlight', - 'text_highlight_minor', 'action_default', 'action'] +COLOR_NAMES = [ + "text_success", + "text_warning", + "text_error", + "text_highlight", + "text_highlight_minor", + "action_default", + "action", + # New Colors + "text", + "text_faint", + "import_path", + "import_path_items", + "action_description", + "added", + "removed", + "changed", + "added_highlight", + "removed_highlight", + "changed_highlight", + "text_diff_added", + "text_diff_removed", + "text_diff_changed", +] COLORS = None def _colorize(color, text): """Returns a string that prints the given text in the given color - in a terminal that is ANSI color-aware. The color must be something - in DARK_COLORS or LIGHT_COLORS. + in a terminal that is ANSI color-aware. The color must be a list of strings + from ANSI_CODES. """ - if color in DARK_COLORS: - escape = COLOR_ESCAPE + "%im" % (DARK_COLORS[color] + 30) - elif color in LIGHT_COLORS: - escape = COLOR_ESCAPE + "%i;01m" % (LIGHT_COLORS[color] + 30) - else: - raise ValueError('no such color %s', color) + # Construct escape sequence to be put before the text by iterating + # over all "ANSI codes" in `color`. + escape = "" + for code in color: + escape = escape + COLOR_ESCAPE + "%im" % ANSI_CODES[code] return escape + text + RESET_COLOR -def colorize(color_name, text): +def colorize(color_name, text, whitespace=True): """Colorize text if colored output is enabled. (Like _colorize but conditional.) """ - if not config['ui']['color'] or 'NO_COLOR' in os.environ.keys(): + if config["ui"]["color"]: + global COLORS + if not COLORS: + # Read all color configurations and set global variable COLORS. + COLORS = dict() + for name in COLOR_NAMES: + # Convert legacy color definitions (strings) into the new + # list-based color definitions. Do this by trying to read the + # color definition from the configuration as unicode - if this + # is successful, the color definition is a legacy definition + # and has to be converted. + try: + color_def = config["ui"]["colors"][name].get(unicode) + except (confuse.ConfigTypeError, NameError): + # Normal color definition (type: list of unicode). + color_def = config["ui"]["colors"][name].get(list) + else: + # Legacy color definition (type: unicode). Convert. + if color_def in LEGACY_COLORS: + color_def = LEGACY_COLORS[color_def] + else: + raise UserError("no such color %s", color_def) + for code in color_def: + if code not in ANSI_CODES.keys(): + raise ValueError("no such ANSI code %s", code) + COLORS[name] = color_def + # In case a 3rd party plugin is still passing the actual color ('red') + # instead of the abstract color name ('text_error') + color = COLORS.get(color_name) + if not color: + log.debug("Invalid color_name: {0}", color_name) + color = color_name + if whitespace: + # Colorize including whitespaces + return _colorize(color, text) + else: + # Split into words, then colorize individually + return " ".join(_colorize(color, word) + for word in text.split()) + else: return text - global COLORS - if not COLORS: - COLORS = {name: - config['ui']['colors'][name].as_str() - for name in COLOR_NAMES} - # In case a 3rd party plugin is still passing the actual color ('red') - # instead of the abstract color name ('text_error') - color = COLORS.get(color_name) - if not color: - log.debug('Invalid color_name: {0}', color_name) - color = color_name - return _colorize(color, text) + +def uncolorize(colored_text): + """Remove colors from a string.""" + # Define a regular expression to match ANSI codes. + # See: http://stackoverflow.com/a/2187024/1382707 + # Explanation of regular expression: + # \x1b - matches ESC character + # \[ - matches opening square bracket + # [;\d]* - matches a sequence consisting of one or more digits or + # semicola + # [A-Za-z] - matches a letter + ansi_code_regex = re.compile(r"\x1b\[[;\d]*[A-Za-z]", re.VERBOSE) + # Strip ANSI codes from `colored_text` using the regular expression. + text = ansi_code_regex.sub("", colored_text) + return text -def _colordiff(a, b, highlight='text_highlight', - minor_highlight='text_highlight_minor'): +def color_len(colored_text): + """Measure the length of a string while excluding ANSI codes from the + measurement. The standard `len(my_string)` method also counts ANSI codes + to the string length, which is counterproductive when layouting a + Terminal interface. + """ + # Return the length of the uncolored string. + return len(uncolorize(colored_text)) + + +def _colordiff(a, b): """Given two values, return the same pair of strings except with their differences highlighted in the specified color. Strings are highlighted intelligently to show differences; other values are stringified and highlighted in their entirety. """ - if not isinstance(a, str) \ - or not isinstance(b, str): - # Non-strings: use ordinary equality. - a = str(a) - b = str(b) - if a == b: - return a, b - else: - return colorize(highlight, a), colorize(highlight, b) - + # First, convert paths to readable format if isinstance(a, bytes) or isinstance(b, bytes): # A path field. a = util.displayable_path(a) b = util.displayable_path(b) + if not isinstance(a, str) or not isinstance(b, str): + # Non-strings: use ordinary equality. + if a == b: + return str(a), str(b) + else: + return ( + colorize("text_diff_removed", str(a)), + colorize("text_diff_added", str(b)) + ) + a_out = [] b_out = [] @@ -566,32 +682,41 @@ def _colordiff(a, b, highlight='text_highlight', a_out.append(a[a_start:a_end]) b_out.append(b[b_start:b_end]) elif op == 'insert': - # Right only. - b_out.append(colorize(highlight, b[b_start:b_end])) + # Right only. Colorize whitespace if added. + b_out.append(colorize("text_diff_added", + b[b_start:b_end], + whitespace=True)) elif op == 'delete': # Left only. - a_out.append(colorize(highlight, a[a_start:a_end])) + a_out.append(colorize("text_diff_removed", + a[a_start:a_end], + whitespace=False)) elif op == 'replace': # Right and left differ. Colorise with second highlight if # it's just a case change. if a[a_start:a_end].lower() != b[b_start:b_end].lower(): - color = highlight + a_color = "text_diff_removed" + b_color = "text_diff_added" else: - color = minor_highlight - a_out.append(colorize(color, a[a_start:a_end])) - b_out.append(colorize(color, b[b_start:b_end])) + a_color = b_color = "text_highlight_minor" + a_out.append(colorize(a_color, + a[a_start:a_end], + whitespace=False)) + b_out.append(colorize(b_color, + b[b_start:b_end], + whitespace=False)) else: assert False return ''.join(a_out), ''.join(b_out) -def colordiff(a, b, highlight='text_highlight'): +def colordiff(a, b): """Colorize differences between two values if color is enabled. (Like _colordiff but conditional.) """ if config['ui']['color']: - return _colordiff(a, b, highlight) + return _colordiff(a, b) else: return str(a), str(b) @@ -648,6 +773,258 @@ def term_width(): return width +def split_into_lines(string, width_tuple): + """Splits string into a list of substrings at whitespace. + + `width_tuple` is a 3-tuple of `(first_width, last_width, middle_width)`. + The first substring has a length not longer than `first_width`, the last + substring has a length not longer than `last_width`, and all other + substrings have a length not longer than `middle_width`. + `string` may contain ANSI codes at word borders. + """ + first_width, middle_width, last_width = width_tuple + words = [] + if uncolorize(string) == string: + # No colors in string + words = string.split() + else: + # Use a regex to find escapes and the text within them. + esc_text = re.compile(r"(?P\x1b\[[;\d]*[A-Za-z])" + r"(?P[^\x1b]+)", re.VERBOSE) + for m in esc_text.finditer(string): + # m contains two groups: + # esc - intitial escape sequence + # text - text, no escape sequence, may contain spaces + raw_words = m.group("text").split() + # Reconstruct colored words, without spaces. + words += [m.group("esc") + raw_word + + RESET_COLOR for raw_word in raw_words] + result = [] + next_substr = "" + # Iterate over all words. + for i in range(len(words)): + if i == 0: + pot_substr = words[i] + else: + pot_substr = " ".join([next_substr, words[i]]) + # Find out if the pot(ential)_substr fits into the next substring. + fits_first = ( + len(result) == 0 and color_len(pot_substr) <= first_width + ) + fits_middle = ( + len(result) != 0 and color_len(pot_substr) <= middle_width + ) + if fits_first or fits_middle: + next_substr = pot_substr + else: + result.append(next_substr) + next_substr = words[i] + # We finished constructing the substrings, but the last substring + # has not yet been added to the result. + result.append(next_substr) + # Also, the length of the last substring was only checked against + # `middle_width`. Append an empty substring as the new last substring if + # the last substring is too long. + if not color_len(next_substr) <= last_width: + result.append('') + return result + + +def print_column_layout( + indent_str, left, right, separator=" -> ", max_width=term_width() +): + """Print left & right data, with separator inbetween + 'left' and 'right' have a structure of: + {'prefix':u'','contents':u'','suffix':u'','width':0} + In a column layout the printing will be: + {indent_str}{lhs0}{separator}{rhs0} + {lhs1 / padding }{rhs1} + ... + The first line of each column (i.e. {lhs0} or {rhs0}) is: + {prefix}{part of contents}{suffix} + With subsequent lines (i.e. {lhs1}, {rhs1} onwards) being the + rest of contents, wrapped if the width would be otherwise exceeded. + """ + if right["prefix"] + right["contents"] + right["suffix"] == '': + # No right hand information, so we don't need a separator. + separator = "" + first_line_no_wrap = ( + indent_str + + left["prefix"] + + left["contents"] + + left["suffix"] + + separator + + right["prefix"] + + right["contents"] + + right["suffix"] + ) + if color_len(first_line_no_wrap) < max_width: + # Everything fits, print out line. + print_(first_line_no_wrap) + else: + # Wrap into columns + if "width" not in left or "width" not in right: + # If widths have not been defined, set to share space. + left["width"] = (max_width - len(indent_str) + - color_len(separator)) // 2 + right["width"] = (max_width - len(indent_str) + - color_len(separator)) // 2 + # On the first line, account for suffix as well as prefix + left_width_tuple = ( + left["width"] - color_len(left["prefix"]) + - color_len(left["suffix"]), + left["width"] - color_len(left["prefix"]), + left["width"] - color_len(left["prefix"]), + ) + + left_split = split_into_lines(left["contents"], left_width_tuple) + right_width_tuple = ( + right["width"] - color_len(right["prefix"]) + - color_len(right["suffix"]), + right["width"] - color_len(right["prefix"]), + right["width"] - color_len(right["prefix"]), + ) + + right_split = split_into_lines(right["contents"], right_width_tuple) + max_line_count = max(len(left_split), len(right_split)) + + out = "" + for i in range(max_line_count): + # indentation + out += indent_str + + # Prefix or indent_str for line + if i == 0: + out += left["prefix"] + else: + out += indent(color_len(left["prefix"])) + + # Line i of left hand side contents. + if i < len(left_split): + out += left_split[i] + left_part_len = color_len(left_split[i]) + else: + left_part_len = 0 + + # Padding until end of column. + # Note: differs from original + # column calcs in not -1 afterwards for space + # in track number as that is included in 'prefix' + padding = left["width"] - color_len(left["prefix"]) - left_part_len + + # Remove some padding on the first line to display + # length + if i == 0: + padding -= color_len(left["suffix"]) + + out += indent(padding) + + if i == 0: + out += left["suffix"] + + # Separator between columns. + if i == 0: + out += separator + else: + out += indent(color_len(separator)) + + # Right prefix, contents, padding, suffix + if i == 0: + out += right["prefix"] + else: + out += indent(color_len(right["prefix"])) + + # Line i of right hand side. + if i < len(right_split): + out += right_split[i] + right_part_len = color_len(right_split[i]) + else: + right_part_len = 0 + + # Padding until end of column + padding = right["width"] - color_len(right["prefix"]) \ + - right_part_len + # Remove some padding on the first line to display + # length + if i == 0: + padding -= color_len(right["suffix"]) + out += indent(padding) + # Length in first line + if i == 0: + out += right["suffix"] + + # Linebreak, except in the last line. + if i < max_line_count - 1: + out += "\n" + + # Constructed all of the columns, now print + print_(out) + + +def print_newline_layout( + indent_str, left, right, separator=" -> ", max_width=term_width() +): + """Prints using a newline separator between left & right if + they go over their allocated widths. The datastructures are + shared with the column layout. In contrast to the column layout, + the prefix and suffix are printed at the beginning and end of + the contents. If no wrapping is required (i.e. everything fits) the + first line will look exactly the same as the column layout: + {indent}{lhs0}{separator}{rhs0} + However if this would go over the width given, the layout now becomes: + {indent}{lhs0} + {indent}{separator}{rhs0} + If {lhs0} would go over the maximum width, the subsequent lines are + indented a second time for ease of reading. + """ + if right["prefix"] + right["contents"] + right["suffix"] == '': + # No right hand information, so we don't need a separator. + separator = "" + first_line_no_wrap = ( + indent_str + + left["prefix"] + + left["contents"] + + left["suffix"] + + separator + + right["prefix"] + + right["contents"] + + right["suffix"] + ) + if color_len(first_line_no_wrap) < max_width: + # Everything fits, print out line. + print_(first_line_no_wrap) + else: + # Newline separation, with wrapping + empty_space = max_width - len(indent_str) + # On lower lines we will double the indent for clarity + left_width_tuple = ( + empty_space, + empty_space - len(indent_str), + empty_space - len(indent_str), + ) + left_str = left["prefix"] + left["contents"] + left["suffix"] + left_split = split_into_lines(left_str, left_width_tuple) + # Repeat calculations for rhs, including separator on first line + right_width_tuple = ( + empty_space - color_len(separator), + empty_space - len(indent_str), + empty_space - len(indent_str), + ) + right_str = right["prefix"] + right["contents"] + right["suffix"] + right_split = split_into_lines(right_str, right_width_tuple) + for i, line in enumerate(left_split): + if i == 0: + print_(indent_str + line) + elif line != "": + # Ignore empty lines + print_(indent_str * 2 + line) + for i, line in enumerate(right_split): + if i == 0: + print_(indent_str + separator + line) + elif line != "": + print_(indent_str * 2 + line) + + FLOAT_EPSILON = 0.01 diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 8f68d319b..093787135 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -26,7 +26,8 @@ from typing import Sequence import beets from beets import ui -from beets.ui import print_, input_, decargs, show_path_changes +from beets.ui import print_, input_, decargs, show_path_changes, \ + print_newline_layout, print_column_layout from beets import autotag from beets.autotag import Recommendation from beets.autotag import hooks @@ -160,7 +161,6 @@ default_commands.append(fields_cmd) # help: Print help text for commands class HelpCommand(ui.Subcommand): - def __init__(self): super().__init__( 'help', aliases=('?',), @@ -243,18 +243,25 @@ def get_album_disambig_fields(info: hooks.AlbumInfo) -> Sequence[str]: return out +def dist_colorize(string, dist): + """Formats a string as a colorized similarity string according to + a distance. + """ + if dist <= config["match"]["strong_rec_thresh"].as_number(): + string = ui.colorize("text_success", string) + elif dist <= config["match"]["medium_rec_thresh"].as_number(): + string = ui.colorize("text_warning", string) + else: + string = ui.colorize("text_error", string) + return string + + def dist_string(dist): """Formats a distance (a float) as a colorized similarity percentage string. """ - out = '%.1f%%' % ((1 - dist) * 100) - if dist <= config['match']['strong_rec_thresh'].as_number(): - out = ui.colorize('text_success', out) - elif dist <= config['match']['medium_rec_thresh'].as_number(): - out = ui.colorize('text_warning', out) - else: - out = ui.colorize('text_error', out) - return out + string = "{:.1f}%".format(((1 - dist) * 100)) + return dist_colorize(string, dist) def penalty_string(distance, limit=None): @@ -270,24 +277,172 @@ def penalty_string(distance, limit=None): if penalties: if limit and len(penalties) > limit: penalties = penalties[:limit] + ['...'] - return ui.colorize('text_warning', '(%s)' % ', '.join(penalties)) + # Prefix penalty string with U+2260: Not Equal To + penalty_string = "\u2260 {}".format(", ".join(penalties)) + return ui.colorize("changed", penalty_string) -def show_change(cur_artist, cur_album, match): - """Print out a representation of the changes that will be made if an - album's tags are changed according to `match`, which must be an AlbumMatch - object. +class ChangeRepresentation(object): + """Keeps track of all information needed to generate a (colored) text + representation of the changes that will be made if an album or singleton's + tags are changed according to `match`, which must be an AlbumMatch or + TrackMatch object, accordingly. """ - def show_album(artist, album): - if artist: - album_description = f' {artist} - {album}' - elif album: - album_description = ' %s' % album - else: - album_description = ' (unknown album)' - print_(album_description) - def format_index(track_info): + cur_artist = None + # cur_album set if album, cur_title set if singleton + cur_album = None + cur_title = None + match = None + indent_header = "" + indent_detail = "" + + def __init__(self): + # Read match header indentation width from config. + match_header_indent_width = config["ui"]["import"]["indentation"][ + "match_header" + ].as_number() + self.indent_header = ui.indent(match_header_indent_width) + + # Read match detail indentation width from config. + match_detail_indent_width = config["ui"]["import"]["indentation"][ + "match_details" + ].as_number() + self.indent_detail = ui.indent(match_detail_indent_width) + + # Read match tracklist indentation width from config + match_tracklist_indent_width = config["ui"]["import"]["indentation"][ + "match_tracklist" + ].as_number() + self.indent_tracklist = ui.indent(match_tracklist_indent_width) + self.layout = config["ui"]["import"]["layout"].as_choice( + { + "column": 0, + "newline": 1, + } + ) + + def print_layout(self, indent, left, right, separator=" -> ", + max_width=None): + if not max_width: + # If no max_width provided, use terminal width + max_width = ui.term_width() + if self.layout == 0: + print_column_layout(indent, left, right, separator, max_width) + else: + print_newline_layout(indent, left, right, separator, max_width) + + def show_match_header(self): + """Print out a 'header' identifying the suggested match (album name, + artist name,...) and summarizing the changes that would be made should + the user accept the match. + """ + # Print newline at beginning of change block. + print_("") + + # 'Match' line and similarity. + print_(self.indent_header + + f"Match ({dist_string(self.match.distance)}):") + + if self.match.info.get("album"): + # Matching an album - print that + artist_album_str = f"{self.match.info.artist}" + \ + f" - {self.match.info.album}" + else: + # Matching a single track + artist_album_str = f"{self.match.info.artist}" + \ + f" - {self.match.info.title}" + print_( + self.indent_header + + dist_colorize(artist_album_str, self.match.distance) + ) + + # Penalties. + penalties = penalty_string(self.match.distance) + if penalties: + print_(self.indent_header + penalties) + + # Disambiguation. + disambig = disambig_string(self.match.info) + if disambig: + print_(self.indent_header + disambig) + + # Data URL. + if self.match.info.data_url: + url = ui.colorize("text_faint", f"{self.match.info.data_url}") + print_(self.indent_header + url) + + def show_match_details(self): + """Print out the details of the match, including changes in album name + and artist name. + """ + # Artist. + artist_l, artist_r = self.cur_artist or "", self.match.info.artist + if artist_r == VARIOUS_ARTISTS: + # Hide artists for VA releases. + artist_l, artist_r = "", "" + if artist_l != artist_r: + artist_l, artist_r = ui.colordiff(artist_l, artist_r) + # Prefix with U+2260: Not Equal To + left = { + "prefix": ui.colorize("changed", "\u2260") + " Artist: ", + "contents": artist_l, + "suffix": "", + } + right = {"prefix": "", "contents": artist_r, "suffix": ""} + self.print_layout(self.indent_detail, left, right) + + else: + print_(self.indent_detail + "*", "Artist:", artist_r) + + if self.cur_album: + # Album + album_l, album_r = self.cur_album or "", self.match.info.album + if ( + self.cur_album != self.match.info.album + and self.match.info.album != VARIOUS_ARTISTS + ): + album_l, album_r = ui.colordiff(album_l, album_r) + # Prefix with U+2260: Not Equal To + left = { + "prefix": ui.colorize("changed", "\u2260") + " Album: ", + "contents": album_l, + "suffix": "", + } + right = {"prefix": "", "contents": album_r, "suffix": ""} + self.print_layout(self.indent_detail, left, right) + else: + print_(self.indent_detail + "*", "Album:", album_r) + elif self.cur_title: + # Title - for singletons + title_l, title_r = self.cur_title or "", self.match.info.title + if self.cur_title != self.match.info.title: + title_l, title_r = ui.colordiff(title_l, title_r) + # Prefix with U+2260: Not Equal To + left = { + "prefix": ui.colorize("changed", "\u2260") + " Title: ", + "contents": title_l, + "suffix": "", + } + right = {"prefix": "", "contents": title_r, "suffix": ""} + self.print_layout(self.indent_detail, left, right) + else: + print_(self.indent_detail + "*", "Title:", title_r) + + def make_medium_info_line(self, track_info): + """Construct a line with the current medium's info.""" + media = self.match.info.media or "Media" + # Build output string. + if self.match.info.mediums > 1 and track_info.disctitle: + return f"* {media} {track_info.medium}: {track_info.disctitle}" + elif self.match.info.mediums > 1: + return f"* {media} {track_info.medium}" + elif track_info.disctitle: + return f"* {media}: {track_info.disctitle}" + else: + return "" + + def format_index(self, track_info): """Return a string representing the track index of the given TrackInfo or Item object. """ @@ -295,7 +450,7 @@ def show_change(cur_artist, cur_album, match): index = track_info.index medium_index = track_info.medium_index medium = track_info.medium - mediums = match.info.mediums + mediums = self.match.info.mediums else: index = medium_index = track_info.track medium = track_info.disc @@ -309,205 +464,271 @@ def show_change(cur_artist, cur_album, match): else: return str(index) - # Identify the album in question. - if cur_artist != match.info.artist or \ - (cur_album != match.info.album and - match.info.album != VARIOUS_ARTISTS): - artist_l, artist_r = cur_artist or '', match.info.artist - album_l, album_r = cur_album or '', match.info.album - if artist_r == VARIOUS_ARTISTS: - # Hide artists for VA releases. - artist_l, artist_r = '', '' - - if config['artist_credit']: - artist_r = match.info.artist_credit - - artist_l, artist_r = ui.colordiff(artist_l, artist_r) - album_l, album_r = ui.colordiff(album_l, album_r) - - print_("Correcting tags from:") - show_album(artist_l, album_l) - print_("To:") - show_album(artist_r, album_r) - else: - print_("Tagging:\n {0.artist} - {0.album}".format(match.info)) - - # Data URL. - if match.info.data_url: - print_('URL:\n %s' % match.info.data_url) - - # Info line. - info = [] - # Similarity. - info.append('(Similarity: %s)' % dist_string(match.distance)) - # Penalties. - penalties = penalty_string(match.distance) - if penalties: - info.append(penalties) - # Disambiguation. - disambig = disambig_string(match.info) - if disambig: - info.append(ui.colorize('text_highlight_minor', '(%s)' % disambig)) - print_(' '.join(info)) - - # Tracks. - pairs = list(match.mapping.items()) - pairs.sort(key=lambda item_and_track_info: item_and_track_info[1].index) - - # Build up LHS and RHS for track difference display. The `lines` list - # contains ``(lhs, rhs, width)`` tuples where `width` is the length (in - # characters) of the uncolorized LHS. - lines = [] - medium = disctitle = None - for item, track_info in pairs: - - # Medium number and title. - if medium != track_info.medium or disctitle != track_info.disctitle: - media = match.info.media or 'Media' - if match.info.mediums > 1 and track_info.disctitle: - lhs = '{} {}: {}'.format(media, track_info.medium, - track_info.disctitle) - elif match.info.mediums > 1: - lhs = f'{media} {track_info.medium}' - elif track_info.disctitle: - lhs = f'{media}: {track_info.disctitle}' + def make_track_numbers(self, item, track_info): + """Format colored track indices.""" + cur_track = self.format_index(item) + new_track = self.format_index(track_info) + templ = "(#{})" + changed = False + # Choose color based on change. + if cur_track != new_track: + changed = True + if item.track in (track_info.index, track_info.medium_index): + highlight_color = "text_highlight_minor" else: - lhs = None - if lhs: - lines.append((lhs, '', 0)) - medium, disctitle = track_info.medium, track_info.disctitle + highlight_color = "text_highlight" + else: + highlight_color = "text_faint" - # Titles. + cur_track = templ.format(cur_track) + new_track = templ.format(new_track) + lhs_track = ui.colorize(highlight_color, cur_track) + rhs_track = ui.colorize(highlight_color, new_track) + return lhs_track, rhs_track, changed + + @staticmethod + def make_track_titles(item, track_info): + """Format colored track titles.""" new_title = track_info.title if not item.title.strip(): - # If there's no title, we use the filename. + # If there's no title, we use the filename. Don't colordiff. cur_title = displayable_path(os.path.basename(item.path)) - lhs, rhs = cur_title, new_title + return cur_title, new_title, True else: + # If there is a title, highlight differences. cur_title = item.title.strip() - lhs, rhs = ui.colordiff(cur_title, new_title) - lhs_width = len(cur_title) + cur_col, new_col = ui.colordiff(cur_title, new_title) + return cur_col, new_col, cur_title != new_title + @staticmethod + def make_track_lengths(item, track_info): + """Format colored track lengths.""" + changed = False + if ( + item.length + and track_info.length + and abs(item.length - track_info.length) + > config["ui"]["length_diff_thresh"].as_number() + ): + highlight_color = "text_highlight" + changed = True + else: + highlight_color = "text_highlight_minor" + + # Handle nonetype lengths by setting to 0 + cur_length0 = item.length if item.length else 0 + new_length0 = track_info.length if track_info.length else 0 + # format into string + cur_length = f"({ui.human_seconds_short(cur_length0)})" + new_length = f"({ui.human_seconds_short(new_length0)})" + # colorize + lhs_length = ui.colorize(highlight_color, cur_length) + rhs_length = ui.colorize(highlight_color, new_length) + + return lhs_length, rhs_length, changed + + def make_line(self, item, track_info): + """Extract changes from item -> new TrackInfo object, and colorize + appropriately. Returns (lhs, rhs) for column printing. + """ + # Track titles. + lhs_title, rhs_title, diff_title = \ + self.make_track_titles(item, track_info) # Track number change. - cur_track, new_track = format_index(item), format_index(track_info) - if cur_track != new_track: - if item.track in (track_info.index, track_info.medium_index): - color = 'text_highlight_minor' - else: - color = 'text_highlight' - templ = ui.colorize(color, ' (#{0})') - lhs += templ.format(cur_track) - rhs += templ.format(new_track) - lhs_width += len(cur_track) + 4 - + lhs_track, rhs_track, diff_track = \ + self.make_track_numbers(item, track_info) # Length change. - if item.length and track_info.length and \ - abs(item.length - track_info.length) > \ - config['ui']['length_diff_thresh'].as_number(): - cur_length = ui.human_seconds_short(item.length) - new_length = ui.human_seconds_short(track_info.length) - templ = ui.colorize('text_highlight', ' ({0})') - lhs += templ.format(cur_length) - rhs += templ.format(new_length) - lhs_width += len(cur_length) + 3 + lhs_length, rhs_length, diff_length = \ + self.make_track_lengths(item, track_info) - # Penalties. - penalties = penalty_string(match.distance.tracks[track_info]) - if penalties: - rhs += ' %s' % penalties + changed = diff_title or diff_track or diff_length - if lhs != rhs: - lines.append((' * %s' % lhs, rhs, lhs_width)) - elif config['import']['detail']: - lines.append((' * %s' % lhs, '', lhs_width)) + # Construct lhs and rhs dicts. + # Previously, we printed the penalties, however this is no longer + # the case, thus the 'info' dictionary is unneeded. + # penalties = penalty_string(self.match.distance.tracks[track_info]) - # Print each track in two columns, or across two lines. - col_width = (ui.term_width() - len(''.join([' * ', ' -> ']))) // 2 - if lines: - max_width = max(w for _, _, w in lines) - for lhs, rhs, lhs_width in lines: - if not rhs: - print_(lhs) - elif max_width > col_width: - print_(f'{lhs} ->\n {rhs}') - else: - pad = max_width - lhs_width - print_('{}{} -> {}'.format(lhs, ' ' * pad, rhs)) + prefix = ui.colorize("changed", "\u2260 ") if changed else "* " + lhs = { + "prefix": prefix + lhs_track + " ", + "contents": lhs_title, + "suffix": " " + lhs_length, + } + rhs = {"prefix": "", "contents": "", "suffix": ""} + if not changed: + # Only return the left side, as nothing changed. + return (lhs, rhs) + else: + # Construct a dictionary for the "changed to" side + rhs = { + "prefix": rhs_track + " ", + "contents": rhs_title, + "suffix": " " + rhs_length, + } + return (lhs, rhs) - # Missing and unmatched tracks. - if match.extra_tracks: - print_('Missing tracks ({}/{} - {:.1%}):'.format( - len(match.extra_tracks), - len(match.info.tracks), - len(match.extra_tracks) / len(match.info.tracks) - )) - pad_width = max(len(track_info.title) for track_info in - match.extra_tracks) - for track_info in match.extra_tracks: - line = ' ! {0: <{width}} (#{1: >2})'.format(track_info.title, - format_index(track_info), - width=pad_width) - if track_info.length: - line += ' (%s)' % ui.human_seconds_short(track_info.length) - print_(ui.colorize('text_warning', line)) - if match.extra_items: - print_('Unmatched tracks ({}):'.format(len(match.extra_items))) - pad_width = max(len(item.title) for item in match.extra_items) - for item in match.extra_items: - line = ' ! {0: <{width}} (#{1: >2})'.format(item.title, - format_index(item), - width=pad_width) - if item.length: - line += ' (%s)' % ui.human_seconds_short(item.length) - print_(ui.colorize('text_warning', line)) + def print_tracklist(self, lines): + """Calculates column widths for tracks stored as line tuples: + (left, right). Then prints each line of tracklist. + """ + if len(lines) == 0: + # If no lines provided, e.g. details not required, do nothing. + return + + def get_width(side): + """Return the width of left or right in uncolorized characters.""" + try: + return len( + ui.uncolorize( + " ".join([side["prefix"], + side["contents"], + side["suffix"]]) + ) + ) + except KeyError: + # An empty dictionary -> Nothing to report + return 0 + + # Check how to fit content into terminal window + indent_width = len(self.indent_tracklist) + terminal_width = ui.term_width() + joiner_width = len("".join(["* ", " -> "])) + col_width = (terminal_width - indent_width - joiner_width) // 2 + max_width_l = max(get_width(line_tuple[0]) for line_tuple in lines) + max_width_r = max(get_width(line_tuple[1]) for line_tuple in lines) + + if ( + (max_width_l <= col_width) + and (max_width_r <= col_width) + or ( + ((max_width_l > col_width) or (max_width_r > col_width)) + and ((max_width_l + max_width_r) <= col_width * 2) + ) + ): + # All content fits. Either both maximum widths are below column + # widths, or one of the columns is larger than allowed but the + # other is smaller than allowed. + # In this case we can afford to shrink the columns to fit their + # largest string + col_width_l = max_width_l + col_width_r = max_width_r + else: + # Not all content fits - stick with original half/half split + col_width_l = col_width + col_width_r = col_width + + # Print out each line, using the calculated width from above. + for left, right in lines: + left["width"] = col_width_l + right["width"] = col_width_r + self.print_layout(self.indent_tracklist, left, right) + + +class AlbumChange(ChangeRepresentation): + """Album change representation, setting cur_album""" + + def __init__(self, cur_artist, cur_album, match): + super(AlbumChange, self).__init__() + self.cur_artist = cur_artist + self.cur_album = cur_album + self.match = match + + def show_match_tracks(self): + """Print out the tracks of the match, summarizing changes the match + suggests for them. + """ + # Tracks. + # match is an AlbumMatch named tuple, mapping is a dict + # Sort the pairs by the track_info index (at index 1 of the namedtuple) + pairs = list(self.match.mapping.items()) + pairs.sort( + key=lambda item_and_track_info: item_and_track_info[1].index) + # Build up LHS and RHS for track difference display. The `lines` list + # contains `(left, right)` tuples. + lines = [] + medium = disctitle = None + for item, track_info in pairs: + # If the track is the first on a new medium, show medium + # number and title. + if medium != track_info.medium or \ + disctitle != track_info.disctitle: + # Create header for new medium + header = self.make_medium_info_line(track_info) + if header != "": + # Print tracks from previous medium + self.print_tracklist(lines) + lines = [] + print_(self.indent_detail + header) + # Save new medium details for future comparison. + medium, disctitle = track_info.medium, track_info.disctitle + + if config["import"]["detail"]: + # Construct the line tuple for the track. + left, right = self.make_line(item, track_info) + lines.append((left, right)) + self.print_tracklist(lines) + + # Missing and unmatched tracks. + if self.match.extra_tracks: + print_( + "Missing tracks ({0}/{1} - {2:.1%}):".format( + len(self.match.extra_tracks), + len(self.match.info.tracks), + len(self.match.extra_tracks) / len(self.match.info.tracks), + ) + ) + for track_info in self.match.extra_tracks: + line = f" ! {track_info.title} (#{self.format_index(track_info)})" + if track_info.length: + line += f" ({ui.human_seconds_short(track_info.length)})" + print_(ui.colorize("text_warning", line)) + if self.match.extra_items: + print_(f"Unmatched tracks ({len(self.match.extra_items)}):") + for item in self.match.extra_items: + line = " ! {} (#{})".format(item.title, self.format_index(item)) + if item.length: + line += " ({})".format(ui.human_seconds_short(item.length)) + print_(ui.colorize("text_warning", line)) + + +class TrackChange(ChangeRepresentation): + """Track change representation, comparing item with match.""" + + def __init__(self, cur_artist, cur_title, match): + super(TrackChange, self).__init__() + self.cur_artist = cur_artist + self.cur_title = cur_title + self.match = match + + +def show_change(cur_artist, cur_album, match): + """Print out a representation of the changes that will be made if an + album's tags are changed according to `match`, which must be an AlbumMatch + object. + """ + change = AlbumChange(cur_artist=cur_artist, cur_album=cur_album, + match=match) + + # Print the match header. + change.show_match_header() + + # Print the match details. + change.show_match_details() + + # Print the match tracks. + change.show_match_tracks() def show_item_change(item, match): """Print out the change that would occur by tagging `item` with the metadata from `match`, a TrackMatch object. """ - cur_artist, new_artist = item.artist, match.info.artist - cur_title, new_title = item.title, match.info.title - cur_album = item.album if item.album else "" - new_album = match.info.album if match.info.album else "" - - if (cur_artist != new_artist or cur_title != new_title - or cur_album != new_album): - cur_artist, new_artist = ui.colordiff(cur_artist, new_artist) - cur_title, new_title = ui.colordiff(cur_title, new_title) - cur_album, new_album = ui.colordiff(cur_album, new_album) - - print_("Correcting track tags from:") - print_(f" {cur_artist} - {cur_title}") - if cur_album: - print_(f" Album: {cur_album}") - print_("To:") - print_(f" {new_artist} - {new_title}") - if new_album: - print_(f" Album: {new_album}") - - else: - print_(f"Tagging track: {cur_artist} - {cur_title}") - if cur_album: - print_(f" Album: {new_album}") - - # Data URL. - if match.info.data_url: - print_('URL:\n %s' % match.info.data_url) - - # Info line. - info = [] - # Similarity. - info.append('(Similarity: %s)' % dist_string(match.distance)) - # Penalties. - penalties = penalty_string(match.distance) - if penalties: - info.append(penalties) - # Disambiguation. - disambig = disambig_string(match.info) - if disambig: - info.append(ui.colorize('text_highlight_minor', '(%s)' % disambig)) - print_(' '.join(info)) + change = TrackChange(cur_artist=item.artist, cur_title=item.title, + match=match) + # Print the match header. + change.show_match_header() + # Print the match details. + change.show_match_details() def summarize_items(items, singleton): @@ -640,36 +861,40 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, if not bypass_candidates: # Display list of candidates. + print_("") print_('Finding tags for {} "{} - {}".'.format( 'track' if singleton else 'album', item.artist if singleton else cur_artist, item.title if singleton else cur_album, )) - print_('Candidates:') + print_(ui.indent(2) + 'Candidates:') for i, match in enumerate(candidates): # Index, metadata, and distance. - line = [ - '{}.'.format(i + 1), - '{} - {}'.format( - match.info.artist, - match.info.title if singleton else match.info.album, - ), - '({})'.format(dist_string(match.distance)), - ] + index0 = "{0}.".format(i + 1) + index = dist_colorize(index0, match.distance) + dist = "({:.1f}%)".format((1 - match.distance) * 100) + distance = dist_colorize(dist, match.distance) + metadata = "{0} - {1}".format( + match.info.artist, + match.info.title if singleton else match.info.album, + ) + if i == 0: + metadata = dist_colorize(metadata, match.distance) + else: + metadata = ui.colorize("text_highlight_minor", metadata) + line1 = [index, distance, metadata] + print_(ui.indent(2) + " ".join(line1)) # Penalties. penalties = penalty_string(match.distance, 3) if penalties: - line.append(penalties) + print_(ui.indent(13) + penalties) # Disambiguation disambig = disambig_string(match.info) if disambig: - line.append(ui.colorize('text_highlight_minor', - '(%s)' % disambig)) - - print_(' '.join(line)) + print_(ui.indent(13) + disambig) # Ask the user for a choice. sel = ui.input_options(choice_opts, @@ -769,8 +994,12 @@ class TerminalImportSession(importer.ImportSession): """ # Show what we're tagging. print_() - print_(displayable_path(task.paths, '\n') + - ' ({} items)'.format(len(task.items))) + + path_str0 = displayable_path(task.paths, '\n') + path_str = ui.colorize('import_path', path_str0) + items_str0 = '({} items)'.format(len(task.items)) + items_str = ui.colorize('import_path_items', items_str0) + print_(' '.join([path_str, items_str])) # Let plugins display info or prompt the user before we go through the # process of selecting candidate. diff --git a/test/test_ui.py b/test/test_ui.py index 27d36c493..e4b134f4b 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -1184,59 +1184,114 @@ class ShowChangeTest(_common.TestCase): ] ) - def _show_change(self, items=None, info=None, + def _show_change(self, items=None, info=None, color=False, cur_artist='the artist', cur_album='the album', dist=0.1): """Return an unicode string representing the changes""" items = items or self.items info = info or self.info mapping = dict(zip(items, info.tracks)) - config['ui']['color'] = False - album_dist = distance(items, info, mapping) - album_dist._penalties = {'album': [dist]} + config['ui']['color'] = color + config['import']['detail'] = True + change_dist = distance(items, info, mapping) + change_dist._penalties = {'album': [dist], 'artist': [dist]} commands.show_change( cur_artist, cur_album, - autotag.AlbumMatch(album_dist, info, mapping, set(), set()), + autotag.AlbumMatch(change_dist, info, mapping, set(), set()), ) return self.io.getoutput().lower() def test_null_change(self): msg = self._show_change() - self.assertTrue('similarity: 90' in msg) - self.assertTrue('tagging:' in msg) + self.assertTrue('match (90.0%)' in msg) + self.assertTrue('album, artist' in msg) def test_album_data_change(self): msg = self._show_change(cur_artist='another artist', cur_album='another album') - self.assertTrue('correcting tags from:' in msg) + self.assertTrue('another artist -> the artist' in msg) + self.assertTrue('another album -> the album' in msg) def test_item_data_change(self): self.items[0].title = 'different' msg = self._show_change() - self.assertTrue('different -> the title' in msg) + self.assertTrue('different' in msg and 'the title' in msg) def test_item_data_change_with_unicode(self): self.items[0].title = 'caf\xe9' msg = self._show_change() - self.assertTrue('caf\xe9 -> the title' in msg) + self.assertTrue(u'caf\xe9' in msg and 'the title' in msg) def test_album_data_change_with_unicode(self): - msg = self._show_change(cur_artist='caf\xe9', - cur_album='another album') - self.assertTrue('correcting tags from:' in msg) + msg = self._show_change(cur_artist=u'caf\xe9', + cur_album=u'another album') + self.assertTrue(u'caf\xe9' in msg and 'the artist' in msg) def test_item_data_change_title_missing(self): self.items[0].title = '' msg = re.sub(r' +', ' ', self._show_change()) - self.assertTrue('file.mp3 -> the title' in msg) + self.assertTrue(u'file.mp3' in msg and 'the title' in msg) def test_item_data_change_title_missing_with_unicode_filename(self): self.items[0].title = '' self.items[0].path = '/path/to/caf\xe9.mp3'.encode() msg = re.sub(r' +', ' ', self._show_change()) - self.assertTrue('caf\xe9.mp3 -> the title' in msg or - 'caf.mp3 ->' in msg) + self.assertTrue(u'caf\xe9.mp3' in msg or + u'caf.mp3' in msg) + + def test_split_into_lines(self): + # Test uncolored text + txt = ui.split_into_lines("test test test", [5, 5, 5]) + self.assertEqual(txt, ["test", "test", "test"]) + # Test multiple colored texts + colored_text = "\x1b[31mtest \x1b[39;49;00m" * 3 + split_txt = ["\x1b[31mtest\x1b[39;49;00m", + "\x1b[31mtest\x1b[39;49;00m", + "\x1b[31mtest\x1b[39;49;00m"] + txt = ui.split_into_lines(colored_text, [5, 5, 5]) + self.assertEqual(txt, split_txt) + # Test single color, multi space text + colored_text = "\x1b[31m test test test \x1b[39;49;00m" + txt = ui.split_into_lines(colored_text, [5, 5, 5]) + self.assertEqual(txt, split_txt) + + def test_album_data_change_wrap_newline(self): + # Patch ui.term_width to force wrapping + with patch('beets.ui.commands.ui.term_width', return_value=30): + # Test newline layout + config['ui']['import']['layout'] = u'newline' + long_name = u'another artist with a' + (u' very' * 10) + \ + u' long name' + msg = self._show_change(cur_artist=long_name, + cur_album='another album') + # _common.log.info("Message:{}".format(msg)) + self.assertTrue('artist: another artist' in msg) + self.assertTrue(' -> the artist' in msg) + self.assertFalse('another album -> the album' in msg) + + def test_item_data_change_wrap_column(self): + # Patch ui.term_width to force wrapping + with patch('beets.ui.commands.ui.term_width', return_value=54): + # Test Column layout + config['ui']['import']['layout'] = u'column' + long_title = u'a track with a' + (u' very' * 10) + \ + u' long name' + self.items[0].title = long_title + msg = self._show_change() + self.assertTrue('(#1) a track (1:00) -> (#1) the title (0:00)' + in msg) + + def test_item_data_change_wrap_newline(self): + # Patch ui.term_width to force wrapping + with patch('beets.ui.commands.ui.term_width', return_value=30): + config['ui']['import']['layout'] = u'newline' + long_title = u'a track with a' + (u' very' * 10) + \ + u' long name' + self.items[0].title = long_title + msg = self._show_change() + self.assertTrue('(#1) a track with' in msg) + self.assertTrue(' -> (#1) the title (0:00)' in msg) @patch('beets.library.Item.try_filesize', Mock(return_value=987)) From 1249380767278de39ca169b3f8486a2ea2db5c86 Mon Sep 17 00:00:00 2001 From: David Swarbrick Date: Sun, 26 Mar 2023 19:44:29 +0000 Subject: [PATCH 02/14] Fix handling of whitespace near color codes Improve the split_into_lines regex and whitespace handling so that spaces are handled and colored text can be wrapped Create a new test suite for the color splitting function as it was previously introducing rogue escape characters when splitting colorized words. --- beets/ui/__init__.py | 152 ++++++++++++++++++++++++++++++++++++------- test/test_ui.py | 26 ++++++++ 2 files changed, 154 insertions(+), 24 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 37131b203..44aef0ed8 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -577,7 +577,7 @@ def _colorize(color, text): return escape + text + RESET_COLOR -def colorize(color_name, text, whitespace=True): +def colorize(color_name, text): """Colorize text if colored output is enabled. (Like _colorize but conditional.) """ @@ -613,13 +613,7 @@ def colorize(color_name, text, whitespace=True): if not color: log.debug("Invalid color_name: {0}", color_name) color = color_name - if whitespace: - # Colorize including whitespaces - return _colorize(color, text) - else: - # Split into words, then colorize individually - return " ".join(_colorize(color, word) - for word in text.split()) + return _colorize(color, text) else: return text @@ -640,6 +634,43 @@ def uncolorize(colored_text): return text +def color_split(colored_text, index): + ansi_code_regex = re.compile(r"(\x1b\[[;\d]*[A-Za-z])", re.VERBOSE) + length = 0 + pre_split = "" + post_split = "" + found_color_code = None + found_split = False + for part in ansi_code_regex.split(colored_text): + # Count how many real letters we have passed + length += color_len(part) + if found_split: + post_split += part + else: + if ansi_code_regex.match(part): + # This is a color code + if part == RESET_COLOR: + found_color_code = None + else: + found_color_code = part + pre_split += part + else: + if index < length: + # Found part with our split in. + split_index = index - (length - color_len(part)) + found_split = True + if found_color_code: + pre_split += part[:split_index] + RESET_COLOR + post_split += found_color_code + part[split_index:] + else: + pre_split += part[:split_index] + post_split += part[split_index:] + else: + # Not found, add this part to the pre split + pre_split += part + return pre_split, post_split + + def color_len(colored_text): """Measure the length of a string while excluding ANSI codes from the measurement. The standard `len(my_string)` method also counts ANSI codes @@ -682,15 +713,13 @@ def _colordiff(a, b): a_out.append(a[a_start:a_end]) b_out.append(b[b_start:b_end]) elif op == 'insert': - # Right only. Colorize whitespace if added. + # Right only. b_out.append(colorize("text_diff_added", - b[b_start:b_end], - whitespace=True)) + b[b_start:b_end])) elif op == 'delete': # Left only. a_out.append(colorize("text_diff_removed", - a[a_start:a_end], - whitespace=False)) + a[a_start:a_end])) elif op == 'replace': # Right and left differ. Colorise with second highlight if # it's just a case change. @@ -700,11 +729,9 @@ def _colordiff(a, b): else: a_color = b_color = "text_highlight_minor" a_out.append(colorize(a_color, - a[a_start:a_end], - whitespace=False)) + a[a_start:a_end])) b_out.append(colorize(b_color, - b[b_start:b_end], - whitespace=False)) + b[b_start:b_end])) else: assert False @@ -784,28 +811,76 @@ def split_into_lines(string, width_tuple): """ first_width, middle_width, last_width = width_tuple words = [] + esc_text = re.compile(r"""(?P[^\x1b]*) + (?P(?:\x1b\[[;\d]*[A-Za-z])+) + (?P[^\x1b]+)(?P\x1b\[39;49;00m) + (?P[^\x1b]*)""", + re.VERBOSE) if uncolorize(string) == string: # No colors in string words = string.split() else: # Use a regex to find escapes and the text within them. - esc_text = re.compile(r"(?P\x1b\[[;\d]*[A-Za-z])" - r"(?P[^\x1b]+)", re.VERBOSE) for m in esc_text.finditer(string): - # m contains two groups: + # m contains four groups: + # pretext - any text before escape sequence # esc - intitial escape sequence # text - text, no escape sequence, may contain spaces + # reset - ASCII colour reset + space_before_text = False + if m.group("pretext") != "": + # Some pretext found, let's handle it + # Add any words in the pretext + words += m.group("pretext").split() + if m.group("pretext")[-1] == " ": + # Pretext ended on a space + space_before_text = True + else: + # Pretext ended mid-word, ensure next word + pass + else: + # pretext empty, treat as if there is a space before + space_before_text = True + if m.group("text")[0] == " ": + # First character of the text is a space + space_before_text = True + # Now, handle the words in the main text: raw_words = m.group("text").split() - # Reconstruct colored words, without spaces. - words += [m.group("esc") + raw_word - + RESET_COLOR for raw_word in raw_words] + if space_before_text: + # Colorize each word with pre/post escapes + # Reconstruct colored words + words += [m.group("esc") + raw_word + + RESET_COLOR for raw_word in raw_words] + else: + # Pretext stops mid-word + if m.group("esc") != RESET_COLOR: + # Add the rest of the current word, with a reset after it + words[-1] += m.group("esc") + raw_words[0] + RESET_COLOR + # Add the subsequent colored words: + words += [m.group("esc") + raw_word + + RESET_COLOR for raw_word in raw_words[1:]] + else: + # Caught a mid-word escape sequence + words[-1] += raw_words[0] + words += raw_words[1:] + if (m.group("text")[-1] != " " and m.group("posttext") != "" + and m.group("posttext")[0] != " "): + # reset falls mid-word + post_text = m.group("posttext").split() + words[-1] += post_text[0] + words += post_text[1:] + else: + # Add any words after escape sequence + words += m.group("posttext").split() result = [] next_substr = "" # Iterate over all words. + previous_fit = False for i in range(len(words)): if i == 0: pot_substr = words[i] else: + # (optimistically) add the next word to check the fit pot_substr = " ".join([next_substr, words[i]]) # Find out if the pot(ential)_substr fits into the next substring. fits_first = ( @@ -815,10 +890,39 @@ def split_into_lines(string, width_tuple): len(result) != 0 and color_len(pot_substr) <= middle_width ) if fits_first or fits_middle: + # Fitted(!) let's try and add another word before appending next_substr = pot_substr - else: + previous_fit = True + elif not fits_first and not fits_middle and previous_fit: + # Extra word didn't fit, append what we have result.append(next_substr) next_substr = words[i] + previous_fit = color_len(next_substr) <= middle_width + else: + # Didn't fit anywhere + if uncolorize(pot_substr) == pot_substr: + # Simple uncolored string, append a cropped word + if len(result) == 0: + # Crop word by the first_width for the first line + result.append(pot_substr[:first_width]) + # add rest of word to next line + next_substr = pot_substr[first_width:] + else: + result.append(pot_substr[:middle_width]) + next_substr = pot_substr[middle_width:] + else: + # Colored strings + if len(result) == 0: + this_line, next_line = color_split(pot_substr, first_width) + result.append(this_line) + next_substr = next_line + else: + this_line, next_line = color_split(pot_substr, + middle_width) + result.append(this_line) + next_substr = next_line + previous_fit = color_len(next_substr) <= middle_width + # We finished constructing the substrings, but the last substring # has not yet been added to the result. result.append(next_substr) diff --git a/test/test_ui.py b/test/test_ui.py index e4b134f4b..f8ecd5d49 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -1240,6 +1240,25 @@ class ShowChangeTest(_common.TestCase): self.assertTrue(u'caf\xe9.mp3' in msg or u'caf.mp3' in msg) + def test_colorize(self): + self.assertEqual("test", ui.uncolorize("test")) + txt = ui.uncolorize("\x1b[31mtest\x1b[39;49;00m") + self.assertEqual("test", txt) + txt = ui.uncolorize("\x1b[31mtest\x1b[39;49;00m test") + self.assertEqual("test test", txt) + txt = ui.uncolorize("\x1b[31mtest\x1b[39;49;00mtest") + self.assertEqual("testtest", txt) + txt = ui.uncolorize("test \x1b[31mtest\x1b[39;49;00m test") + self.assertEqual("test test test", txt) + + def test_color_split(self): + exp = ("test", "") + res = ui.color_split("test", 5) + self.assertEqual(exp, res) + exp = ("\x1b[31mtes\x1b[39;49;00m", "\x1b[31mt\x1b[39;49;00m") + res = ui.color_split("\x1b[31mtest\x1b[39;49;00m", 3) + self.assertEqual(exp, res) + def test_split_into_lines(self): # Test uncolored text txt = ui.split_into_lines("test test test", [5, 5, 5]) @@ -1255,6 +1274,13 @@ class ShowChangeTest(_common.TestCase): colored_text = "\x1b[31m test test test \x1b[39;49;00m" txt = ui.split_into_lines(colored_text, [5, 5, 5]) self.assertEqual(txt, split_txt) + # Test single color, different spacing + colored_text = "\x1b[31mtest\x1b[39;49;00mtest test test" + # ToDo: fix color_len to handle mid-text color escapes, and thus + # split colored texts over newlines (potentially with dashes?) + split_txt = ["\x1b[31mtest\x1b[39;49;00mt", "est", "test", "test"] + txt = ui.split_into_lines(colored_text, [5, 5, 5]) + self.assertEqual(txt, split_txt) def test_album_data_change_wrap_newline(self): # Patch ui.term_width to force wrapping From b4a5e0b3a76b8b01d74fd2500e72b6efb71abbe0 Mon Sep 17 00:00:00 2001 From: David Swarbrick Date: Sat, 2 Sep 2023 15:24:20 +0100 Subject: [PATCH 03/14] Change default config to improve readability on PuTTY --- beets/config_default.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/beets/config_default.yaml b/beets/config_default.yaml index 114e9b6e8..a20797208 100644 --- a/beets/config_default.yaml +++ b/beets/config_default.yaml @@ -103,11 +103,11 @@ ui: text_highlight: ['bold', 'red'] text_highlight_minor: ['white'] action_default: ['bold', 'cyan'] - action: ['bold', 'blue'] + action: ['bold', 'cyan'] # New Colors text: ['normal'] text_faint: ['faint'] - import_path: ['bold', 'inverse', 'blue'] + import_path: ['bold', 'blue'] import_path_items: ['bold', 'blue'] added: ['green'] removed: ['red'] @@ -118,7 +118,7 @@ ui: text_diff_added: ['bold', 'red'] text_diff_removed: ['bold', 'red'] text_diff_changed: ['bold', 'red'] - action_description: ['blue'] + action_description: ['white'] import: indentation: match_header: 2 From 7ce3b417fc5540b39e084477a0d87691b748aa64 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Mon, 9 Oct 2023 10:41:58 +0200 Subject: [PATCH 04/14] Fix legacy colors config sanity check in ui package was using Python2 type 'unicode' --- beets/ui/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 44aef0ed8..815565d5a 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -593,7 +593,7 @@ def colorize(color_name, text): # is successful, the color definition is a legacy definition # and has to be converted. try: - color_def = config["ui"]["colors"][name].get(unicode) + color_def = config["ui"]["colors"][name].get(str) except (confuse.ConfigTypeError, NameError): # Normal color definition (type: list of unicode). color_def = config["ui"]["colors"][name].get(list) From 5d500223b3a1a52d3e295acb51d5613fae1a6492 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Mon, 9 Oct 2023 11:25:23 +0200 Subject: [PATCH 05/14] Draft docs update for UI code-overhaul --- docs/reference/config.rst | 62 ++++++++++++++++++++++++++++++++++----- 1 file changed, 54 insertions(+), 8 deletions(-) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 56ed835a1..6d9d17866 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -427,20 +427,66 @@ the ``color`` option is set to ``yes``. For example, you might have a section in your configuration file that looks like this:: ui: - color: yes colors: - text_success: green - text_warning: yellow - text_error: red - text_highlight: red - text_highlight_minor: lightgray - action_default: turquoise - action: blue + text_success: ['bold', 'green'] + text_warning: ['bold', 'yellow'] + text_error: ['bold', 'red'] + text_highlight: ['bold', 'red'] + text_highlight_minor: ['white'] + action_default: ['bold', 'cyan'] + action: ['bold', 'cyan'] + # New colors after UI overhaul + text: ['normal'] + text_faint: ['faint'] + import_path: ['bold', 'blue'] + import_path_items: ['bold', 'blue'] + added: ['green'] + removed: ['red'] + changed: ['yellow'] + added_highlight: ['bold', 'green'] + removed_highlight: ['bold', 'red'] + changed_highlight: ['bold', 'yellow'] + text_diff_added: ['bold', 'red'] + text_diff_removed: ['bold', 'red'] + text_diff_changed: ['bold', 'red'] + action_description: ['white'] Available colors: black, darkred, darkgreen, brown (darkyellow), darkblue, purple (darkmagenta), teal (darkcyan), lightgray, darkgray, red, green, yellow, blue, fuchsia (magenta), turquoise (cyan), white +Legacy UI colors config directive used strings. If any colors value is still a +string instead of a list, it will be translated to list automatically. For +example ``blue`` will become ``['blue']``. + +terminal_width +~~~~~~~~~~~~~~ + +Controls line wrapping. Defaults to ``80`` characters:: + + ui: + terminal_width: 80 + +length_diff_thresh +~~~~~~~~~~~~~~~~~~ + +FIXME:: + + ui: + length_diff_thresh: 10.0 + +import +~~~~~~ + +FIXME:: + + ui: + import: + indentation: + match_header: 2 + match_details: 2 + match_tracklist: 5 + layout: column Importer Options ---------------- From d1166c8efc22d1f331d0ee96c2d0c80715008629 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Thu, 12 Oct 2023 18:17:25 +0200 Subject: [PATCH 06/14] Add length_diff_thresh description and ref-links to config reference docs. --- docs/reference/config.rst | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 6d9d17866..32a4cb8bd 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -419,6 +419,8 @@ support ANSI colors. still respected, but a deprecation message will be shown until your top-level `color` configuration has been nested under `ui`. +.. _colors: + colors ~~~~~~ @@ -470,7 +472,13 @@ Controls line wrapping. Defaults to ``80`` characters:: length_diff_thresh ~~~~~~~~~~~~~~~~~~ -FIXME:: +Beets compares the length of the imported track with the length the metadata +source provides. If any tracks differ by at least ``length_diff_thresh`` +seconds, they will be colored with ``text_highlight``. Below this threshold, +different track lengths are colored with ``text_highlight_minor``. +``length_diff_thresh`` does not impact which releases are selected in +autotagger matching or distance score calculation (see :ref:`match-config` and +:ref:`colors`):: ui: length_diff_thresh: 10.0 From d875fee722748741b4b1056c6552bd872c20aa86 Mon Sep 17 00:00:00 2001 From: Serene-Arc <33189705+Serene-Arc@users.noreply.github.com> Date: Sat, 14 Oct 2023 15:53:28 +1000 Subject: [PATCH 07/14] Add text for docs section --- docs/reference/config.rst | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 32a4cb8bd..e5d6d111d 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -486,7 +486,13 @@ autotagger matching or distance score calculation (see :ref:`match-config` and import ~~~~~~ -FIXME:: +When importing, beets will read several options to configure the visuals of the +import dialogue. There are two layouts: ``column`` and ``newline``. The +indentation of the respective elements of the import UI can also be configured +to highlight specific sections, in character units e.g. a ``5`` will indent the +given section by five characters in the terminal. + +:: ui: import: From fabfde33c6b51cfd1577d733716800ddc423adbe Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Sat, 14 Oct 2023 08:29:38 +0200 Subject: [PATCH 08/14] Fix track length comparision in UI code Fixes behaviour to what we documented already. --- beets/ui/commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 093787135..7d39c7b9b 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -508,7 +508,7 @@ class ChangeRepresentation(object): item.length and track_info.length and abs(item.length - track_info.length) - > config["ui"]["length_diff_thresh"].as_number() + >= config["ui"]["length_diff_thresh"].as_number() ): highlight_color = "text_highlight" changed = True From 81c10a6f5e00fc74d59aed4a308bde146f8d041b Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Sat, 14 Oct 2023 09:15:18 +0200 Subject: [PATCH 09/14] Improve ui:import description in docs --- docs/reference/config.rst | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index e5d6d111d..cb1c1d44d 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -487,12 +487,11 @@ import ~~~~~~ When importing, beets will read several options to configure the visuals of the -import dialogue. There are two layouts: ``column`` and ``newline``. The -indentation of the respective elements of the import UI can also be configured -to highlight specific sections, in character units e.g. a ``5`` will indent the -given section by five characters in the terminal. - -:: +import dialogue. There are two layouts controlling how horizontal space and +line wrapping is dealt with: ``column`` and ``newline``. The indentation of the +respective elements of the import UI can also be configured. For example +setting ``5`` for ``match_header`` will indent the very first block of a +proposed match by five characters in the terminal.:: ui: import: From 5898736c0ac2e19a6206da8629fd7b75a2975cd2 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Sat, 14 Oct 2023 09:50:07 +0200 Subject: [PATCH 10/14] Provide a working example in ui:import docs Instead of just stating config_default.yaml's values, provide a copy/paste-able example that changes _all_ the values and still kind of works (visually). --- docs/reference/config.rst | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index cb1c1d44d..bc048e502 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -490,16 +490,16 @@ When importing, beets will read several options to configure the visuals of the import dialogue. There are two layouts controlling how horizontal space and line wrapping is dealt with: ``column`` and ``newline``. The indentation of the respective elements of the import UI can also be configured. For example -setting ``5`` for ``match_header`` will indent the very first block of a -proposed match by five characters in the terminal.:: +setting ``4`` for ``match_header`` will indent the very first block of a +proposed match by five characters in the terminal:: ui: import: indentation: - match_header: 2 - match_details: 2 - match_tracklist: 5 - layout: column + match_header: 4 + match_details: 4 + match_tracklist: 7 + layout: newline Importer Options ---------------- From 18dc6dbc7337c5074ab796a5e601d4092c9d9d63 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Sat, 14 Oct 2023 09:52:57 +0200 Subject: [PATCH 11/14] Fix ref-links in ui:length_diff_thresh docs --- docs/reference/config.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index bc048e502..ed799895c 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -477,8 +477,8 @@ source provides. If any tracks differ by at least ``length_diff_thresh`` seconds, they will be colored with ``text_highlight``. Below this threshold, different track lengths are colored with ``text_highlight_minor``. ``length_diff_thresh`` does not impact which releases are selected in -autotagger matching or distance score calculation (see :ref:`match-config` and -:ref:`colors`):: +autotagger matching or distance score calculation (see :ref:`match-config`, +``distance_weights`` and :ref:`colors`):: ui: length_diff_thresh: 10.0 From dfcf256d1ed2f95641e1eecc4cc046773614ece9 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Sat, 14 Oct 2023 10:07:48 +0200 Subject: [PATCH 12/14] Add anchor at "UI options" in docs to make linking to it possible. --- docs/reference/config.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index ed799895c..3f1b4be65 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -398,6 +398,8 @@ Sets the albumartist for various-artist compilations. Defaults to ``'Various Artists'`` (the MusicBrainz standard). Affects other sources, such as :doc:`/plugins/discogs`, too. +.. _ui_options: + UI Options ---------- From 05236d75cfa3d83262f99209f9a68a9924083f97 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Sat, 14 Oct 2023 10:32:37 +0200 Subject: [PATCH 13/14] Changelog for #3721 "UI overhaul" and new section "Major new features:" similar to what we had with 1.6.0. --- docs/changelog.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index a17d1c886..ccd318e9e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,6 +9,12 @@ Changelog goes here! Please add your entry to the bottom of one of the lists bel With this release, beets now requires Python 3.7 or later (it removes support for Python 3.6). +Major new features: + +* The beets importer UI received a major overhaul. Several new configuration + options are available for customizing layout and colors: :ref:`ui_options`. + :bug:`3721` + New features: * :ref:`update-cmd`: added ```-e``` flag for excluding fields from being updated. From b35f0546d723fe6a834cd18023c95f2376ae5ecf Mon Sep 17 00:00:00 2001 From: Serene-Arc <33189705+Serene-Arc@users.noreply.github.com> Date: Sat, 14 Oct 2023 20:23:47 +1000 Subject: [PATCH 14/14] Remove old configuration option --- beets/config_default.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/config_default.yaml b/beets/config_default.yaml index a20797208..b2d345aec 100644 --- a/beets/config_default.yaml +++ b/beets/config_default.yaml @@ -197,5 +197,5 @@ match: ignore_video_tracks: yes track_length_grace: 10 track_length_max: 30 - album_disambig_fields: data_source media year country label catalognum albumdisambig albumrelease + album_disambig_fields: data_source media year country label catalognum albumdisambig singleton_disambig_fields: data_source index track_alt album