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

@@ -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

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()})

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))

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".""",