From 9a11a58404006e429e9a4f98d4f7e49e42631c51 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Wed, 20 Dec 2023 00:30:57 +0100 Subject: [PATCH 1/6] edit plugin: make temporary file reflect --set CLI arguments --- beetsplug/edit.py | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/beetsplug/edit.py b/beetsplug/edit.py index 323dd9e417..b6ae2ba4cc 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -23,7 +23,7 @@ import yaml -from beets import plugins, ui, util +from beets import plugins, ui, util, config from beets.dbcore import types from beets.importer import action from beets.ui.commands import PromptChoice, _do_query @@ -155,9 +155,14 @@ def __init__(self): } ) + self.readonly_fields = {} + self.register_listener( "before_choose_candidate", self.before_choose_candidate_listener ) + self.register_listener( + "import_begin", self.import_begin_listener + ) def commands(self): edit_command = ui.Subcommand("edit", help="interactively edit metadata") @@ -236,11 +241,22 @@ def edit_objects(self, objs, fields): # Get the content to edit as raw data structures. old_data = [flatten(o, fields) for o in objs] + # take set fields into account + if self.readonly_fields: + old_str = "# note: the following fields will be reset to their current values:\n" + for key in self.readonly_fields: + old_str += f"# - {key}\n" + for obj in old_data: + # those values will be enforced later anyway + obj.update(self.readonly_fields) + else: + old_str = "" + # Set up a temporary file with the initial data for editing. new = NamedTemporaryFile( mode="w", suffix=".yaml", delete=False, encoding="utf-8" ) - old_str = dump(old_data) + old_str += dump(old_data) new.write(old_str) new.close() @@ -358,6 +374,12 @@ def before_choose_candidate_listener(self, session, task): return choices + def import_begin_listener(self, session): + """Load data from import session into self""" + self.readonly_fields = {} + for field, view in config["import"]["set_fields"].items(): + self.readonly_fields[field] = view.get() + def importer_edit(self, session, task): """Callback for invoking the functionality during an interactive import session on the *original* item tags. From 0d26cc1482ff5080ec579b17b29f22657a20c562 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Sat, 23 Dec 2023 01:37:17 +0100 Subject: [PATCH 2/6] edit plugin: rework set-fields warning message --- beetsplug/edit.py | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/beetsplug/edit.py b/beetsplug/edit.py index b6ae2ba4cc..8a07fccaaf 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -155,14 +155,11 @@ def __init__(self): } ) - self.readonly_fields = {} + self.has_shown_ui = False self.register_listener( "before_choose_candidate", self.before_choose_candidate_listener ) - self.register_listener( - "import_begin", self.import_begin_listener - ) def commands(self): edit_command = ui.Subcommand("edit", help="interactively edit metadata") @@ -242,13 +239,14 @@ def edit_objects(self, objs, fields): old_data = [flatten(o, fields) for o in objs] # take set fields into account - if self.readonly_fields: - old_str = "# note: the following fields will be reset to their current values:\n" - for key in self.readonly_fields: + set_fields = config["import"]["set_fields"] + if set_fields and not self.has_shown_ui: + old_str = "\n\n# note: the following fields will be reset to their current values:\n" + for key in set_fields: old_str += f"# - {key}\n" for obj in old_data: # those values will be enforced later anyway - obj.update(self.readonly_fields) + obj.update({k:v.get() for k,v in set_fields.items()}) else: old_str = "" @@ -256,7 +254,7 @@ def edit_objects(self, objs, fields): new = NamedTemporaryFile( mode="w", suffix=".yaml", delete=False, encoding="utf-8" ) - old_str += dump(old_data) + old_str = dump(old_data) + old_str new.write(old_str) new.close() @@ -265,6 +263,7 @@ def edit_objects(self, objs, fields): while True: # Ask the user to edit the data. edit(new.name, self._log) + self.has_shown_ui = True # Read the data back after editing and check whether anything # changed. @@ -374,12 +373,6 @@ def before_choose_candidate_listener(self, session, task): return choices - def import_begin_listener(self, session): - """Load data from import session into self""" - self.readonly_fields = {} - for field, view in config["import"]["set_fields"].items(): - self.readonly_fields[field] = view.get() - def importer_edit(self, session, task): """Callback for invoking the functionality during an interactive import session on the *original* item tags. From 3a8c5b763ee5b124f722e101d135d2f65504e991 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Fri, 29 Dec 2023 18:14:09 +0100 Subject: [PATCH 3/6] edit plugin: rework set-fields behavior again --- beetsplug/edit.py | 75 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 11 deletions(-) diff --git a/beetsplug/edit.py b/beetsplug/edit.py index 8a07fccaaf..4f0ec314f5 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -18,6 +18,7 @@ import codecs import os import shlex +import re import subprocess from tempfile import NamedTemporaryFile @@ -155,8 +156,6 @@ def __init__(self): } ) - self.has_shown_ui = False - self.register_listener( "before_choose_candidate", self.before_choose_candidate_listener ) @@ -238,23 +237,48 @@ def edit_objects(self, objs, fields): # Get the content to edit as raw data structures. old_data = [flatten(o, fields) for o in objs] - # take set fields into account + # the following few paragraphs do the following: + # take set fields into account, by setting their values, commenting them out, and annotating them + # also pruning the "id" and "path" fields, which can only contain placeholders at this time + + # prepare regex matching + # the following regex detects yaml lines setting values for 'id' or 'path' + placeholder_field_detector = re.compile(r"\s*(-\s+)?(id|path):(\s+.*)?") set_fields = config["import"]["set_fields"] - if set_fields and not self.has_shown_ui: - old_str = "\n\n# note: the following fields will be reset to their current values:\n" - for key in set_fields: - old_str += f"# - {key}\n" + if set_fields: + # a similar regex, but for setting values for any field in the set_fields list + ro_field_detector = re.compile(r"\s*(-\s+)?({0:s}):(\s+.*)?".format( + '|'.join(re.escape(key) for key in set_fields.keys()) + )) + + # deal with the values of the set fields for obj in old_data: - # those values will be enforced later anyway obj.update({k:v.get() for k,v in set_fields.items()}) else: - old_str = "" + ro_field_detector = None + # will in theory never be read, this is just to make static analysis happy + + # convert to text, then comment/annotate/prune lines + old_data_lines = dump(old_data).split('\n') + + line_i = 0 + while line_i < len(old_data_lines): + line = old_data_lines[line_i] + if set_fields and ro_field_detector.fullmatch(line): + line = "#" + line + " # [read-only field]" + old_data_lines[line_i] = line + line_i +=1 + elif placeholder_field_detector.fullmatch(line): + del old_data_lines[line_i] + else: + line_i +=1 + + old_str = "\n".join(old_data_lines) # Set up a temporary file with the initial data for editing. new = NamedTemporaryFile( mode="w", suffix=".yaml", delete=False, encoding="utf-8" ) - old_str = dump(old_data) + old_str new.write(old_str) new.close() @@ -263,7 +287,6 @@ def edit_objects(self, objs, fields): while True: # Ask the user to edit the data. edit(new.name, self._log) - self.has_shown_ui = True # Read the data back after editing and check whether anything # changed. @@ -282,6 +305,36 @@ def edit_objects(self, objs, fields): continue else: return False + # this makes the missing fields of new_data 'default to' the values in old_data + new_data = [ old_obj | new_obj for old_obj,new_obj in zip(old_data,new_data)] + + # see if any changes to the data need to be discarded: + for field in list(set_fields.keys()) + ["id", "path"]: + changed = False + for old_obj, new_obj in zip(old_data, new_data): + if old_obj.get(field,'') != new_obj.get(field,''): + changed = True + if field in old_obj: + new_obj[field] = old_obj[field] + else: + del new_obj[field] + if changed: + # TODO: colors? + if field in ('id','path'): + ui.print_(ui.colorize( + "text_warning", + f"NOTICE: the field \"{field:s}\" is read-only " + "because it can only contain placeholder values at this time.\n" + "The values you have manually set will have to be discarded." + )) + else: + ui.print_(ui.colorize( + "text_warning", + f"NOTICE: the field \"{field:s}\" is read-only " + "because a value for it was set through the `--set` " + "command line arguemnt or an equivalent in beets's configuration.\n" + "The manual changes you have made to it will have to be discarded." + )) # Show the changes. # If the objects are not on the DB yet, we need a copy of their From 071518629c7cf1cddda9209554ce8fd5123b1f9d Mon Sep 17 00:00:00 2001 From: niacdoial Date: Fri, 29 Dec 2023 18:27:41 +0100 Subject: [PATCH 4/6] edit plugin: make formatting tools happy --- beetsplug/edit.py | 71 ++++++++++++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/beetsplug/edit.py b/beetsplug/edit.py index 4f0ec314f5..1de5416185 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -238,28 +238,33 @@ def edit_objects(self, objs, fields): old_data = [flatten(o, fields) for o in objs] # the following few paragraphs do the following: - # take set fields into account, by setting their values, commenting them out, and annotating them - # also pruning the "id" and "path" fields, which can only contain placeholders at this time + # take set fields into account, by setting their values, commenting them out, + # and annotating them + # also pruning the "id" and "path" fields, which can only contain placeholders + # at this time # prepare regex matching # the following regex detects yaml lines setting values for 'id' or 'path' placeholder_field_detector = re.compile(r"\s*(-\s+)?(id|path):(\s+.*)?") set_fields = config["import"]["set_fields"] if set_fields: - # a similar regex, but for setting values for any field in the set_fields list - ro_field_detector = re.compile(r"\s*(-\s+)?({0:s}):(\s+.*)?".format( - '|'.join(re.escape(key) for key in set_fields.keys()) - )) + # a similar regex, but for setting values for any field + # in the set_fields list + ro_field_detector = re.compile( + r"\s*(-\s+)?({0:s}):(\s+.*)?".format( + "|".join(re.escape(key) for key in set_fields.keys()) + ) + ) - # deal with the values of the set fields + # deal with the values of the set fields for obj in old_data: - obj.update({k:v.get() for k,v in set_fields.items()}) + obj.update({k: v.get() for k, v in set_fields.items()}) else: ro_field_detector = None # will in theory never be read, this is just to make static analysis happy # convert to text, then comment/annotate/prune lines - old_data_lines = dump(old_data).split('\n') + old_data_lines = dump(old_data).split("\n") line_i = 0 while line_i < len(old_data_lines): @@ -267,11 +272,11 @@ def edit_objects(self, objs, fields): if set_fields and ro_field_detector.fullmatch(line): line = "#" + line + " # [read-only field]" old_data_lines[line_i] = line - line_i +=1 + line_i += 1 elif placeholder_field_detector.fullmatch(line): del old_data_lines[line_i] else: - line_i +=1 + line_i += 1 old_str = "\n".join(old_data_lines) @@ -305,14 +310,18 @@ def edit_objects(self, objs, fields): continue else: return False - # this makes the missing fields of new_data 'default to' the values in old_data - new_data = [ old_obj | new_obj for old_obj,new_obj in zip(old_data,new_data)] + # this makes the missing fields of new_data 'default to' + # the values in old_data + new_data = [ + old_obj | new_obj + for old_obj, new_obj in zip(old_data, new_data) + ] # see if any changes to the data need to be discarded: for field in list(set_fields.keys()) + ["id", "path"]: changed = False for old_obj, new_obj in zip(old_data, new_data): - if old_obj.get(field,'') != new_obj.get(field,''): + if old_obj.get(field, "") != new_obj.get(field, ""): changed = True if field in old_obj: new_obj[field] = old_obj[field] @@ -320,21 +329,27 @@ def edit_objects(self, objs, fields): del new_obj[field] if changed: # TODO: colors? - if field in ('id','path'): - ui.print_(ui.colorize( - "text_warning", - f"NOTICE: the field \"{field:s}\" is read-only " - "because it can only contain placeholder values at this time.\n" - "The values you have manually set will have to be discarded." - )) + if field in ("id", "path"): + ui.print_( + ui.colorize( + "text_warning", + f'NOTICE: the field "{field:s}" is read-only ' + "because it can only contain placeholder values " + "at this time.\nThe values you have manually set " + "will have to be discarded.", + ) + ) else: - ui.print_(ui.colorize( - "text_warning", - f"NOTICE: the field \"{field:s}\" is read-only " - "because a value for it was set through the `--set` " - "command line arguemnt or an equivalent in beets's configuration.\n" - "The manual changes you have made to it will have to be discarded." - )) + ui.print_( + ui.colorize( + "text_warning", + f'NOTICE: the field "{field:s}" is read-only ' + "because a value for it was set through the `--set` " + "command line arguemnt or an equivalent in beets's " + "configuration.\nThe manual changes you have made " + "to it will have to be discarded.", + ) + ) # Show the changes. # If the objects are not on the DB yet, we need a copy of their From 28618b0c00ee34f99bc2c4c1fbcaeeb821e282df Mon Sep 17 00:00:00 2001 From: niacdoial Date: Fri, 29 Dec 2023 18:44:06 +0100 Subject: [PATCH 5/6] edit plugin: restore the ability to delete fields by remving them in text --- beetsplug/edit.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/beetsplug/edit.py b/beetsplug/edit.py index 1de5416185..33abc8f2c2 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -310,17 +310,16 @@ def edit_objects(self, objs, fields): continue else: return False - # this makes the missing fields of new_data 'default to' - # the values in old_data - new_data = [ - old_obj | new_obj - for old_obj, new_obj in zip(old_data, new_data) - ] # see if any changes to the data need to be discarded: for field in list(set_fields.keys()) + ["id", "path"]: changed = False for old_obj, new_obj in zip(old_data, new_data): + if field in old_obj and field not in new_obj: + # only copy 'missing fields' if they are missing + # due to us pruning/commenting them out in the editable + # text + new_obj[field] = old_obj[field] if old_obj.get(field, "") != new_obj.get(field, ""): changed = True if field in old_obj: @@ -344,7 +343,7 @@ def edit_objects(self, objs, fields): ui.colorize( "text_warning", f'NOTICE: the field "{field:s}" is read-only ' - "because a value for it was set through the `--set` " + "because it was set through the `--set` " "command line arguemnt or an equivalent in beets's " "configuration.\nThe manual changes you have made " "to it will have to be discarded.", From f66ab378981a6ee88e5390bf7f8139684f2cfaa5 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Fri, 5 Jan 2024 11:18:04 +0100 Subject: [PATCH 6/6] edit plugin: make effect of recent commits only effect importing --- beetsplug/edit.py | 193 +++++++++++++++++++++++++++------------------- 1 file changed, 115 insertions(+), 78 deletions(-) diff --git a/beetsplug/edit.py b/beetsplug/edit.py index 33abc8f2c2..dffd33a347 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -142,6 +142,112 @@ def apply_(obj, data): obj.set_parse(key, str(value)) +def dump_and_prune_fields(data): + """For a set of "set fields", plus `id` and `path`, make sure that + those fields are presented in a way that makes sense + (this is for import sessions, where fields affected by the `--set` flag are + effectively read-only, and both `id` and `path` can only contain placeholders + for now) + This function takes the data for the editable text, and returns a modified + version of said text. + """ + + # this does the following: + # take set fields into account, by setting their values, commenting them out, + # and annotating them + # also pruning the "id" and "path" fields, which can only contain placeholders + # at this time + + data = [obj.copy() for obj in data] + + # prepare regex matching + # the following regex detects yaml lines setting values for 'id' or 'path' + placeholder_field_detector = re.compile(r"\s*(-\s+)?(id|path):(\s+.*)?") + set_fields = config["import"]["set_fields"] + if set_fields: + # a similar regex, but for setting values for any field + # in the set_fields list + ro_field_detector = re.compile( + r"\s*(-\s+)?({0:s}):(\s+.*)?".format( + "|".join(re.escape(key) for key in set_fields.keys()) + ) + ) + + # deal with the values of the set fields + for obj in data: + obj.update({k: v.get() for k, v in set_fields.items()}) + else: + # will in theory not be read + # this line is just to make static analysis happy + ro_field_detector = None + + # convert to text, then comment/annotate/prune lines (only on import) + data_lines = dump(data).split("\n") + + line_i = 0 + while line_i < len(data_lines): + line = data_lines[line_i] + if set_fields and ro_field_detector.fullmatch(line): + line = "#" + line + " # [read-only field]" + data_lines[line_i] = line + line_i += 1 + elif placeholder_field_detector.fullmatch(line): + del data_lines[line_i] + else: + line_i += 1 + + return "\n".join(data_lines) + + +def fix_warn_ro_fields(old_data, new_data): + """For a set of new data obtained from edited text, + take care of the read-only fields that were commented out or + pruned out of the text prior to editing: + - first re-add the fields that are now missing + - then warned if those fields were changed when the text was edited + """ + + set_fields = config["import"]["set_fields"].keys() + set_fields = list(set_fields) + ["id", "path"] + for field in set_fields: + changed = False + for old_obj, new_obj in zip(old_data, new_data): + if field in old_obj and field not in new_obj: + # only copy 'missing fields' if they are missing + # due to us pruning/commenting them out in the editable + # text + new_obj[field] = old_obj[field] + if old_obj.get(field, "") != new_obj.get(field, ""): + changed = True + if field in old_obj: + new_obj[field] = old_obj[field] + else: + del new_obj[field] + if changed: + if field in ("id", "path"): + ui.print_( + ui.colorize( + "text_warning", + f'NOTICE: the field "{field:s}" is read-only ' + "because it can only contain placeholder values " + "at this time.\nThe values you have manually set " + "will have to be discarded.", + ) + ) + else: + ui.print_( + ui.colorize( + "text_warning", + f'NOTICE: the field "{field:s}" is read-only ' + "because it was set through the `--set` " + "command line arguemnt or an equivalent in beets's " + "configuration.\nThe manual changes you have made " + "to it will have to be discarded.", + ) + ) + return new_data + + class EditPlugin(plugins.BeetsPlugin): def __init__(self): super().__init__() @@ -159,6 +265,7 @@ def __init__(self): self.register_listener( "before_choose_candidate", self.before_choose_candidate_listener ) + self.session_is_import = False def commands(self): edit_command = ui.Subcommand("edit", help="interactively edit metadata") @@ -237,48 +344,10 @@ def edit_objects(self, objs, fields): # Get the content to edit as raw data structures. old_data = [flatten(o, fields) for o in objs] - # the following few paragraphs do the following: - # take set fields into account, by setting their values, commenting them out, - # and annotating them - # also pruning the "id" and "path" fields, which can only contain placeholders - # at this time - - # prepare regex matching - # the following regex detects yaml lines setting values for 'id' or 'path' - placeholder_field_detector = re.compile(r"\s*(-\s+)?(id|path):(\s+.*)?") - set_fields = config["import"]["set_fields"] - if set_fields: - # a similar regex, but for setting values for any field - # in the set_fields list - ro_field_detector = re.compile( - r"\s*(-\s+)?({0:s}):(\s+.*)?".format( - "|".join(re.escape(key) for key in set_fields.keys()) - ) - ) - - # deal with the values of the set fields - for obj in old_data: - obj.update({k: v.get() for k, v in set_fields.items()}) + if self.session_is_import: + old_str = dump_and_prune_fields(old_data) else: - ro_field_detector = None - # will in theory never be read, this is just to make static analysis happy - - # convert to text, then comment/annotate/prune lines - old_data_lines = dump(old_data).split("\n") - - line_i = 0 - while line_i < len(old_data_lines): - line = old_data_lines[line_i] - if set_fields and ro_field_detector.fullmatch(line): - line = "#" + line + " # [read-only field]" - old_data_lines[line_i] = line - line_i += 1 - elif placeholder_field_detector.fullmatch(line): - del old_data_lines[line_i] - else: - line_i += 1 - - old_str = "\n".join(old_data_lines) + old_str = dump(old_data) # Set up a temporary file with the initial data for editing. new = NamedTemporaryFile( @@ -312,43 +381,8 @@ def edit_objects(self, objs, fields): return False # see if any changes to the data need to be discarded: - for field in list(set_fields.keys()) + ["id", "path"]: - changed = False - for old_obj, new_obj in zip(old_data, new_data): - if field in old_obj and field not in new_obj: - # only copy 'missing fields' if they are missing - # due to us pruning/commenting them out in the editable - # text - new_obj[field] = old_obj[field] - if old_obj.get(field, "") != new_obj.get(field, ""): - changed = True - if field in old_obj: - new_obj[field] = old_obj[field] - else: - del new_obj[field] - if changed: - # TODO: colors? - if field in ("id", "path"): - ui.print_( - ui.colorize( - "text_warning", - f'NOTICE: the field "{field:s}" is read-only ' - "because it can only contain placeholder values " - "at this time.\nThe values you have manually set " - "will have to be discarded.", - ) - ) - else: - ui.print_( - ui.colorize( - "text_warning", - f'NOTICE: the field "{field:s}" is read-only ' - "because it was set through the `--set` " - "command line arguemnt or an equivalent in beets's " - "configuration.\nThe manual changes you have made " - "to it will have to be discarded.", - ) - ) + if self.session_is_import: + new_data = fix_warn_ro_fields(old_data, new_data) # Show the changes. # If the objects are not on the DB yet, we need a copy of their @@ -452,6 +486,8 @@ def importer_edit(self, session, task): if not obj._db or obj.id is None: obj.id = -i + self.session_is_import = True + # Present the YAML to the user and let them change it. fields = self._get_fields(album=False, extra=[]) success = self.edit_objects(task.items, fields) @@ -482,4 +518,5 @@ def importer_edit_candidate(self, session, task): task.match = task.candidates[sel - 1] task.apply_metadata() + self.session_is_import = True return self.importer_edit(session, task)