From ef3cfc561caa2250057cfb6c98c9e845ce57090e Mon Sep 17 00:00:00 2001 From: Yoshi Date: Wed, 7 Dec 2022 06:39:57 +0100 Subject: [PATCH] Facilitate hook updating/replacement (#2213) * Facilitate updating of hooks - Add instructions in contributing.md - Change addon_config_editor_will_update_json hook to work with the new hookslib code * Fix typo in docs * Always run replaced hook * Use lowercase list for typing * Forbid defining both a replaced and a legacy hook --- docs/contributing.md | 8 ++++++ pylib/tools/hookslib.py | 54 ++++++++++++++++++++++++++++++++++------ qt/aqt/addons.py | 6 ----- qt/tools/genhooks_gui.py | 2 ++ 4 files changed, 57 insertions(+), 13 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index 2a8717557..1c57df0d6 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -89,6 +89,14 @@ The hook code is automatically generated using the definitions in pylib/tools/genhooks.py and qt/tools/genhooks_gui.py. Adding a new definition in one of those files will update the generated files. +If you want to change an existing hook to, for example, receive an additional +argument, you must leave the existing hook unchanged to preserve backwards +compatibility. Create a new definition for your hook with a similar name and +include the properties `replaces="name_of_old_hook"` and +`replaced_hook_args=["..."]` in the definition of the new hook. If the old hook +has a legacy hook, you must not add the legacy hook to the definition of the +new hook. + ## Translations For information on adding new translatable strings to Anki, please see diff --git a/pylib/tools/hookslib.py b/pylib/tools/hookslib.py index c65b78480..6361c633e 100644 --- a/pylib/tools/hookslib.py +++ b/pylib/tools/hookslib.py @@ -30,6 +30,10 @@ class Hook: legacy_hook: Optional[str] = None # if legacy hook takes no arguments but the new hook does, set this legacy_no_args: bool = False + # if the hook replaces a deprecated one, add its name here + replaces: Optional[str] = None + # arguments that the hook being replaced took + replaced_hook_args: Optional[list[str]] = None # docstring to add to hook class doc: Optional[str] = None @@ -43,9 +47,9 @@ class Hook: types_str = ", ".join(types) return f"Callable[[{types_str}], {self.return_type or 'None'}]" - def arg_names(self) -> list[str]: + def arg_names(self, args: Optional[list[str]]) -> list[str]: names = [] - for arg in self.args or []: + for arg in args or []: if not arg: continue (name, type) = arg.split(":") @@ -108,10 +112,14 @@ class {self.classname()}: # hook name only return f'"{self.legacy_hook}"' else: - return ", ".join([f'"{self.legacy_hook}"'] + self.arg_names()) + return ", ".join([f'"{self.legacy_hook}"'] + self.arg_names(self.args)) + + def replaced_args(self) -> str: + args = ", ".join(self.arg_names(self.replaced_hook_args)) + return f"{self.replaces}({args})" def hook_fire_code(self) -> str: - arg_names = self.arg_names() + arg_names = self.arg_names(self.args) args_including_self = ["self"] + (self.args or []) out = f"""\ def __call__({", ".join(args_including_self)}) -> None: @@ -123,7 +131,23 @@ class {self.classname()}: self._hooks.remove(hook) raise """ - if self.legacy_hook: + if self.replaces and self.legacy_hook: + raise Exception( + f"Hook {self.name} replaces {self.replaces} and " + "must therefore not define a legacy hook." + ) + elif self.replaces: + out += f"""\ + if {self.replaces}.count() > 0: + print( + "The hook {self.replaces} is deprecated.\\n" + "Use {self.name} instead." + ) + {self.replaced_args()} +""" + elif self.legacy_hook: + # don't run legacy hook if replaced hook exists + # otherwise the legacy hook will be run twice out += f"""\ # legacy support anki.hooks.runHook({self.legacy_args()}) @@ -131,7 +155,7 @@ class {self.classname()}: return f"{out}\n\n" def filter_fire_code(self) -> str: - arg_names = self.arg_names() + arg_names = self.arg_names(self.args) args_including_self = ["self"] + (self.args or []) out = f"""\ def __call__({", ".join(args_including_self)}) -> {self.return_type}: @@ -143,7 +167,23 @@ class {self.classname()}: self._hooks.remove(filter) raise """ - if self.legacy_hook: + if self.replaces and self.legacy_hook: + raise Exception( + f"Hook {self.name} replaces {self.replaces} and " + "must therefore not define a legacy hook." + ) + elif self.replaces: + out += f"""\ + if {self.replaces}.count() > 0: + print( + "The hook {self.replaces} is deprecated.\\n" + "Use {self.name} instead." + ) + {arg_names[0]} = {self.replaced_args()} +""" + elif self.legacy_hook: + # don't run legacy hook if replaced hook exists + # otherwise the legacy hook will be run twice out += f"""\ # legacy support {arg_names[0]} = anki.hooks.runFilter({self.legacy_args()}) diff --git a/qt/aqt/addons.py b/qt/aqt/addons.py index 9a6805e82..30fed96a1 100644 --- a/qt/aqt/addons.py +++ b/qt/aqt/addons.py @@ -1602,12 +1602,6 @@ class ConfigEditor(QDialog): def accept(self) -> None: txt = self.form.editor.toPlainText() txt = gui_hooks.addon_config_editor_will_update_json(txt, self.addon) - if gui_hooks.addon_config_editor_will_save_json.count() > 0: - print( - "The hook addon_config_editor_will_save_json is deprecated.\n" - "Use addon_config_editor_will_update_json instead." - ) - txt = gui_hooks.addon_config_editor_will_save_json(txt) try: new_conf = json.loads(txt) jsonschema.validate(new_conf, self.mgr._addon_schema(self.addon)) diff --git a/qt/tools/genhooks_gui.py b/qt/tools/genhooks_gui.py index 03592c734..145de2cb0 100644 --- a/qt/tools/genhooks_gui.py +++ b/qt/tools/genhooks_gui.py @@ -1119,6 +1119,8 @@ gui_hooks.webview_did_inject_style_into_page.append(mytest) name="addon_config_editor_will_update_json", args=["text: str", "addon: str"], return_type="str", + replaces="addon_config_editor_will_save_json", + replaced_hook_args=["text: str"], doc="""Allows changing the text of the json configuration that was received from the user before actually reading it. For example, you can replace new line in strings by some "\\\\n".""",