From dd61389319af780d1c5f1395135f77ed5dada989 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Mon, 13 Jan 2020 13:57:51 +1000 Subject: [PATCH] New type-safe approach to hooks/filters Still todo: - Add separate module for GUI hooks - Update the remaining runHook/runFilter() calls - Document the changes, including defensive registration --- pylib/Makefile | 6 +- pylib/anki/cards.py | 6 +- pylib/anki/collection.py | 3 +- pylib/anki/hooks.py | 85 +++++++++++++++++---- pylib/anki/sched.py | 4 +- pylib/anki/schedv2.py | 4 +- pylib/tests/test_schedv1.py | 4 +- pylib/tests/test_schedv2.py | 4 +- pylib/tools/genhooks.py | 146 ++++++++++++++++++++++++++++++++++++ qt/aqt/main.py | 5 +- qt/aqt/reviewer.py | 7 +- 11 files changed, 243 insertions(+), 31 deletions(-) create mode 100644 pylib/tools/genhooks.py diff --git a/pylib/Makefile b/pylib/Makefile index 100ed2d2d..d9b3d7bea 100644 --- a/pylib/Makefile +++ b/pylib/Makefile @@ -27,7 +27,11 @@ PROTODEPS := $(wildcard ../proto/*.proto) protoc --proto_path=../proto --python_out=anki --mypy_out=anki $(PROTODEPS) @touch $@ -BUILD_STEPS := .build/run-deps .build/dev-deps .build/py-proto anki/buildinfo.py +.build/hooks: tools/genhooks.py + python tools/genhooks.py + @touch $@ + +BUILD_STEPS := .build/run-deps .build/dev-deps .build/py-proto .build/hooks anki/buildinfo.py # Checking ###################### diff --git a/pylib/anki/cards.py b/pylib/anki/cards.py index c1c7cb2d2..e3e0651d9 100644 --- a/pylib/anki/cards.py +++ b/pylib/anki/cards.py @@ -8,8 +8,8 @@ import time from typing import Any, Dict, Optional, Union import anki # pylint: disable=unused-import +from anki import hooks from anki.consts import * -from anki.hooks import runHook from anki.notes import Note from anki.utils import intTime, joinFields, timestampID @@ -87,7 +87,7 @@ class Card: self.usn = self.col.usn() # bug check if self.queue == 2 and self.odue and not self.col.decks.isDyn(self.did): - runHook("odueInvalid") + hooks.run_odue_invalid_hook() assert self.due < 4294967296 self.col.db.execute( """ @@ -119,7 +119,7 @@ insert or replace into cards values self.usn = self.col.usn() # bug checks if self.queue == 2 and self.odue and not self.col.decks.isDyn(self.did): - runHook("odueInvalid") + hooks.run_odue_invalid_hook() assert self.due < 4294967296 self.col.db.execute( """update cards set diff --git a/pylib/anki/collection.py b/pylib/anki/collection.py index 8c61d437a..3a36a47cd 100644 --- a/pylib/anki/collection.py +++ b/pylib/anki/collection.py @@ -16,6 +16,7 @@ from typing import Any, Dict, Iterable, List, Optional, Tuple, Union import anki.find import anki.latex # sets up hook import anki.template +from anki import hooks from anki.cards import Card from anki.consts import * from anki.db import DB @@ -271,7 +272,7 @@ crt=?, mod=?, scm=?, dty=?, usn=?, ls=?, conf=?""", def modSchema(self, check: bool) -> None: "Mark schema modified. Call this first so user can abort if necessary." if not self.schemaChanged(): - if check and not runFilter("modSchema", True): + if check and not hooks.run_mod_schema_filter(proceed=True): raise AnkiError("abortSchemaMod") self.scm = intTime(1000) self.setMod() diff --git a/pylib/anki/hooks.py b/pylib/anki/hooks.py index 398f2f6d6..ad74fee4f 100644 --- a/pylib/anki/hooks.py +++ b/pylib/anki/hooks.py @@ -1,22 +1,77 @@ # Copyright: Ankitects Pty Ltd and contributors # License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html -"""\ -Hooks - hook management and tools for extending Anki -============================================================================== - -To find available hooks, grep for runHook and runFilter in the source code. - -Instrumenting allows you to modify functions that don't have hooks available. -If you call wrap() with pos='around', the original function will not be called -automatically but can be called with _old(). """ +Tools for extending Anki. + +A hook takes a function that does not return a value. + +A filter takes a function that returns its first argument, optionally +modifying it. +""" + +from __future__ import annotations from typing import Any, Callable, Dict, List import decorator -# Hooks +from anki.cards import Card + +# New hook/filter handling +############################################################################## +# The code in this section is automatically generated - any edits you make +# will be lost. To add new hooks, see ../tools/genhooks.py +# +# To use an existing hook such as leech_hook, you would call the following +# in your code: +# +# from anki import hooks +# hooks.leech_hook.append(myfunc) +# +# @@AUTOGEN@@ + +leech_hook: List[Callable[[Card], None]] = [] +mod_schema_filter: List[Callable[[bool], bool]] = [] +odue_invalid_hook: List[Callable[[], None]] = [] + + +def run_leech_hook(card: Card) -> None: + for hook in leech_hook: + try: + hook(card) + except: + # if the hook fails, remove it + leech_hook.remove(hook) + raise + # legacy support + runHook("leech", card) + + +def run_mod_schema_filter(proceed: bool) -> bool: + for filter in mod_schema_filter: + try: + proceed = filter(proceed) + except: + # if the hook fails, remove it + mod_schema_filter.remove(filter) + raise + return proceed + + +def run_odue_invalid_hook() -> None: + for hook in odue_invalid_hook: + try: + hook() + except: + # if the hook fails, remove it + odue_invalid_hook.remove(hook) + raise + + +# @@AUTOGEN@@ + +# Legacy hook handling ############################################################################## _hooks: Dict[str, List[Callable[..., Any]]] = {} @@ -61,10 +116,14 @@ def remHook(hook, func) -> None: hook.remove(func) -# Instrumenting +# Monkey patching ############################################################################## - - +# Please only use this for prototyping or for when hooks are not practical, +# as add-ons that use monkey patching are more likely to break when Anki is +# updated. +# +# If you call wrap() with pos='around', the original function will not be called +# automatically but can be called with _old(). def wrap(old, new, pos="after") -> Callable: "Override an existing function." diff --git a/pylib/anki/sched.py b/pylib/anki/sched.py index 87b3c6f23..98cacf143 100644 --- a/pylib/anki/sched.py +++ b/pylib/anki/sched.py @@ -7,8 +7,8 @@ import time from heapq import * from operator import itemgetter +from anki import hooks from anki.consts import * -from anki.hooks import runHook from anki.lang import _ # from anki.cards import Card @@ -1150,7 +1150,7 @@ did = ?, queue = %s, due = ?, usn = ? where id = ?""" card.odue = card.odid = 0 card.queue = -1 # notify UI - runHook("leech", card) + hooks.run_leech_hook(card) return True # Tools diff --git a/pylib/anki/schedv2.py b/pylib/anki/schedv2.py index a1733498f..1c68a9ba0 100644 --- a/pylib/anki/schedv2.py +++ b/pylib/anki/schedv2.py @@ -14,9 +14,9 @@ from operator import itemgetter from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Union import anki # pylint: disable=unused-import +from anki import hooks from anki.cards import Card from anki.consts import * -from anki.hooks import runHook from anki.lang import _ from anki.rsbackend import SchedTimingToday from anki.utils import fmtTimeSpan, ids2str, intTime @@ -1270,7 +1270,7 @@ where id = ? if a == 0: card.queue = -1 # notify UI - runHook("leech", card) + hooks.run_leech_hook(card) return True return None diff --git a/pylib/tests/test_schedv1.py b/pylib/tests/test_schedv1.py index 3e2fae3ad..e2c662039 100644 --- a/pylib/tests/test_schedv1.py +++ b/pylib/tests/test_schedv1.py @@ -3,8 +3,8 @@ import copy import time +from anki import hooks from anki.consts import STARTING_FACTOR -from anki.hooks import addHook from anki.utils import intTime from tests.shared import getEmptyCol as getEmptyColOrig @@ -373,7 +373,7 @@ def test_reviews(): def onLeech(card): hooked.append(1) - addHook("leech", onLeech) + hooks.leech_hook.append(onLeech) d.sched.answerCard(c, 1) assert hooked assert c.queue == -1 diff --git a/pylib/tests/test_schedv2.py b/pylib/tests/test_schedv2.py index caec5383e..c4e199b40 100644 --- a/pylib/tests/test_schedv2.py +++ b/pylib/tests/test_schedv2.py @@ -3,8 +3,8 @@ import copy import time +from anki import hooks from anki.consts import STARTING_FACTOR -from anki.hooks import addHook from anki.utils import intTime from tests.shared import getEmptyCol as getEmptyColOrig @@ -395,7 +395,7 @@ def test_reviews(): def onLeech(card): hooked.append(1) - addHook("leech", onLeech) + hooks.leech_hook.append(onLeech) d.sched.answerCard(c, 1) assert hooked assert c.queue == -1 diff --git a/pylib/tools/genhooks.py b/pylib/tools/genhooks.py new file mode 100644 index 000000000..b3f43a48c --- /dev/null +++ b/pylib/tools/genhooks.py @@ -0,0 +1,146 @@ +# Copyright: Ankitects Pty Ltd and contributors +# License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html + +""" +Generate code for hook handling, and insert it into anki/hooks.py. + +To add a new hook: +- update the hooks list below +- run 'make develop' +- send a pull request that includes the changes to this file and hooks.py +""" + +import os +import re +from dataclasses import dataclass +from operator import attrgetter +from typing import Optional, List + + +@dataclass +class Hook: + # the name of the hook. _filter or _hook is appending automatically. + name: str + # string of the typed arguments passed to the callback, eg + # "kind: str, val: int" + cb_args: str = "" + # string of the return type. if set, hook is a filter. + return_type: Optional[str] = None + # if add-ons may be relying on the legacy hook name, add it here + legacy_hook: Optional[str] = None + + def callable(self) -> str: + "Convert args into a Callable." + types = [] + for arg in self.cb_args.split(","): + if not arg: + continue + (name, type) = arg.split(":") + types.append(type.strip()) + types_str = ", ".join(types) + return f"Callable[[{types_str}], {self.return_type or 'None'}]" + + def arg_names(self) -> List[str]: + names = [] + for arg in self.cb_args.split(","): + if not arg: + continue + (name, type) = arg.split(":") + names.append(name.strip()) + return names + + def full_name(self) -> str: + return f"{self.name}_{self.kind()}" + + def kind(self) -> str: + if self.return_type is not None: + return "filter" + else: + return "hook" + + def list_code(self) -> str: + return f"""\ +{self.full_name()}: List[{self.callable()}] = [] +""" + + def fire_code(self) -> str: + if self.return_type is not None: + # filter + return self.filter_fire_code() + else: + # hook + return self.hook_fire_code() + + def hook_fire_code(self) -> str: + arg_names = self.arg_names() + out = f"""\ +def run_{self.full_name()}({self.cb_args}) -> None: + for hook in {self.full_name()}: + try: + hook({", ".join(arg_names)}) + except: + # if the hook fails, remove it + {self.full_name()}.remove(hook) + raise +""" + if self.legacy_hook: + args = ", ".join([f'"{self.legacy_hook}"'] + arg_names) + out += f"""\ + # legacy support + runHook({args}) +""" + return out + "\n\n" + + def filter_fire_code(self) -> str: + arg_names = self.arg_names() + out = f"""\ +def run_{self.full_name()}({self.cb_args}) -> {self.return_type}: + for filter in {self.full_name()}: + try: + {arg_names[0]} = filter({", ".join(arg_names)}) + except: + # if the hook fails, remove it + {self.full_name()}.remove(filter) + raise +""" + if self.legacy_hook: + args = ", ".join([f'"{self.legacy_hook}"'] + arg_names) + out += f"""\ + # legacy support + runFilter({args}) +""" + + out += f"""\ + return {arg_names[0]} +""" + return out + "\n\n" + +# Hook list +###################################################################### + +hooks = [ + Hook(name="leech", cb_args="card: Card", legacy_hook="leech"), + Hook(name="odue_invalid"), + Hook(name="mod_schema", cb_args="proceed: bool", return_type="bool") +] + +hooks.sort(key=attrgetter("name")) + +###################################################################### + +tools_dir = os.path.dirname(__file__) +hooks_py = os.path.join(tools_dir, "..", "anki", "hooks.py") + +code = "" +for hook in hooks: + code += hook.list_code() +code += "\n\n" +for hook in hooks: + code += hook.fire_code() + +orig = open(hooks_py).read() +new = re.sub("(?s)# @@AUTOGEN@@.*?# @@AUTOGEN@@\n", f"# @@AUTOGEN@@\n\n{code}# @@AUTOGEN@@\n", orig) + +open(hooks_py, "wb").write(new.encode("utf8")) + +print("Updated hooks.py") \ No newline at end of file diff --git a/qt/aqt/main.py b/qt/aqt/main.py index c041042f0..ad834a61d 100644 --- a/qt/aqt/main.py +++ b/qt/aqt/main.py @@ -24,6 +24,7 @@ import aqt.sound import aqt.stats import aqt.toolbar import aqt.webview +from anki import hooks from anki.collection import _Collection from anki.hooks import addHook, runFilter, runHook from anki.lang import _, ngettext @@ -1153,9 +1154,9 @@ Difference to correct time: %s.""" ########################################################################## def setupHooks(self) -> None: - addHook("modSchema", self.onSchemaMod) + hooks.mod_schema_filter.append(self.onSchemaMod) addHook("remNotes", self.onRemNotes) - addHook("odueInvalid", self.onOdueInvalid) + hooks.odue_invalid_hook.append(self.onOdueInvalid) addHook("mpvWillPlay", self.onMpvWillPlay) addHook("mpvIdleHook", self.onMpvIdle) diff --git a/qt/aqt/reviewer.py b/qt/aqt/reviewer.py index 0e2fa774b..45fb88b1a 100644 --- a/qt/aqt/reviewer.py +++ b/qt/aqt/reviewer.py @@ -11,8 +11,9 @@ import unicodedata as ucd from typing import List import aqt +from anki import hooks from anki.cards import Card -from anki.hooks import addHook, runFilter, runHook +from anki.hooks import runFilter, runHook from anki.lang import _, ngettext from anki.utils import bodyClass, stripHTML from aqt import AnkiQt @@ -30,7 +31,7 @@ from aqt.utils import ( class Reviewer: "Manage reviews. Maintains a separate state." - def __init__(self, mw: AnkiQt): + def __init__(self, mw: AnkiQt) -> None: self.mw = mw self.web = mw.web self.card = None @@ -41,7 +42,7 @@ class Reviewer: self.typeCorrect = None # web init happens before this is set self.state = None self.bottom = aqt.toolbar.BottomBar(mw, mw.bottomWeb) - addHook("leech", self.onLeech) + hooks.leech_hook.append(self.onLeech) def show(self): self.mw.col.reset()