From ce1853167b4425a320e015e688fb2d83933c0394 Mon Sep 17 00:00:00 2001 From: Glutanimate Date: Fri, 3 Jan 2020 16:32:20 +0100 Subject: [PATCH 1/7] Refactor add-on installation error handling Allows extending the installation pathways more easily. In preparation of .ankiaddon file type handling. --- qt/aqt/addons.py | 125 ++++++++++++++++++++++++++++++----------------- 1 file changed, 80 insertions(+), 45 deletions(-) diff --git a/qt/aqt/addons.py b/qt/aqt/addons.py index 72d949a44..31f74f252 100644 --- a/qt/aqt/addons.py +++ b/qt/aqt/addons.py @@ -3,10 +3,11 @@ # License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html import io import json +import os import re import zipfile from collections import defaultdict -from typing import Any, Callable, Dict, Optional +from typing import Any, Callable, Dict, List, NamedTuple, Optional, Tuple from zipfile import ZipFile import jsonschema @@ -37,6 +38,13 @@ from aqt.utils import ( ) +class AddonInstallationResult(NamedTuple): + success: bool + errmsg: Optional[str] = None + name: Optional[str] = None + conflicts: Optional[List[str]] = None + + class AddonManager: ext = ".ankiaddon" @@ -201,14 +209,14 @@ and have been disabled: %(found)s" return {} return manifest - def install(self, file, manifest=None): + def install(self, file, manifest=None) -> AddonInstallationResult: """Install add-on from path or file-like object. Metadata is read from the manifest file, with keys overriden by supplying a 'manifest' dictionary""" try: zfile = ZipFile(file) except zipfile.BadZipfile: - return False, "zip" + return AddonInstallationResult(success=False, errmsg="zip") with zfile: file_manifest = self.readManifestFile(zfile) @@ -216,7 +224,7 @@ and have been disabled: %(found)s" file_manifest.update(manifest) manifest = file_manifest if not manifest: - return False, "manifest" + return AddonInstallationResult(success=False, errmsg="manifest") package = manifest["package"] conflicts = manifest.get("conflicts", []) found_conflicts = self._disableConflicting(package, conflicts) @@ -230,7 +238,9 @@ and have been disabled: %(found)s" meta.update(manifest_meta) self.writeAddonMeta(package, meta) - return True, meta["name"], found_conflicts + return AddonInstallationResult( + success=True, name=meta["name"], conflicts=found_conflicts + ) def _install(self, dir, zfile): # previously installed? @@ -274,40 +284,30 @@ and have been disabled: %(found)s" # Processing local add-on files ###################################################################### - def processPackages(self, paths): + def processPackages(self, paths) -> Tuple[List[str], List[str]]: log = [] errs = [] + self.mw.progress.start(immediate=True) try: for path in paths: base = os.path.basename(path) - ret = self.install(path) - if ret[0] is False: - if ret[1] == "zip": - msg = _("Corrupt add-on file.") - elif ret[1] == "manifest": - msg = _("Invalid add-on manifest.") - else: - msg = "Unknown error: {}".format(ret[1]) - errs.append( - _( - "Error installing %(base)s: %(error)s" - % dict(base=base, error=msg) - ) + result = self.install(path) + + if not result.success: + errs.extend( + self._installationErrorReport(result, base, mode="local") ) else: - log.append(_("Installed %(name)s" % dict(name=ret[1]))) - if ret[2]: - log.append( - _("The following conflicting add-ons were disabled:") - + " " - + " ".join(ret[2]) - ) + log.extend( + self._installationSuccessReport(result, base, mode="local") + ) finally: self.mw.progress.finish() + return log, errs - # Downloading + # Downloading add-ons from AnkiWeb ###################################################################### def downloadIds(self, ids): @@ -324,32 +324,67 @@ and have been disabled: %(found)s" data, fname = ret fname = fname.replace("_", " ") name = os.path.splitext(fname)[0] - ret = self.install( + result = self.install( io.BytesIO(data), manifest={"package": str(n), "name": name, "mod": intTime()}, ) - if ret[0] is False: - if ret[1] == "zip": - msg = _("Corrupt add-on file.") - elif ret[1] == "manifest": - msg = _("Invalid add-on manifest.") - else: - msg = "Unknown error: {}".format(ret[1]) - errs.append( - _("Error downloading %(id)s: %(error)s") % dict(id=n, error=msg) - ) + if not result.success: + errs.extend(self._installationErrorReport(result, n)) else: - log.append(_("Downloaded %(fname)s" % dict(fname=name))) - if ret[2]: - log.append( - _("The following conflicting add-ons were disabled:") - + " " - + " ".join(ret[2]) - ) + log.extend(self._installationSuccessReport(result, n)) self.mw.progress.finish() return log, errs + # Installation messaging + ###################################################################### + + def _installationErrorReport( + self, result: AddonInstallationResult, base: str, mode="download" + ) -> List[str]: + + messages = { + "zip": _("Corrupt add-on file."), + "manifest": _("Invalid add-on manifest."), + } + + if result.errmsg: + msg = messages.get( + result.errmsg, _("Unknown error: {}".format(result.errmsg)) + ) + else: + msg = _("Unknown error") + + if mode == "download": # preserve old format strings for i18n + template = _("Error downloading %(id)s: %(error)s") + else: + template = _("Error installing %(base)s: %(error)s") + + name = result.name or base + + return [template % dict(base=name, id=name, error=msg)] + + def _installationSuccessReport( + self, result: AddonInstallationResult, base: str, mode="download" + ) -> List[str]: + + if mode == "download": # preserve old format strings for i18n + template = _("Downloaded %(fname)s") + else: + template = _("Installed %(name)s") + + name = result.name or base + strings = [template % dict(name=name, fname=name)] + + if result.conflicts: + strings.append( + _("The following conflicting add-ons were disabled:") + + " " + + " ".join(result.conflicts) + ) + + return strings + # Updating ###################################################################### From 00991e8e8e500427deb9310721c9105c9d48339c Mon Sep 17 00:00:00 2001 From: Glutanimate Date: Fri, 3 Jan 2020 17:48:17 +0100 Subject: [PATCH 2/7] Extend showInfo with the ability to add custom buttons Grants more flexibility in user prompt design --- qt/aqt/utils.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/qt/aqt/utils.py b/qt/aqt/utils.py index cca97d8b0..c32c409dc 100644 --- a/qt/aqt/utils.py +++ b/qt/aqt/utils.py @@ -50,7 +50,15 @@ def showCritical(text, parent=None, help="", title="Anki", textFormat=None): return showInfo(text, parent, help, "critical", title=title, textFormat=textFormat) -def showInfo(text, parent=False, help="", type="info", title="Anki", textFormat=None): +def showInfo( + text, + parent=False, + help="", + type="info", + title="Anki", + textFormat=None, + customBtns=None +): "Show a small info window with an OK button." if parent is False: parent = aqt.mw.app.activeWindow() or aqt.mw @@ -70,8 +78,16 @@ def showInfo(text, parent=False, help="", type="info", title="Anki", textFormat= mb.setText(text) mb.setIcon(icon) mb.setWindowTitle(title) - b = mb.addButton(QMessageBox.Ok) - b.setDefault(True) + if customBtns: + default = None + for btn in customBtns: + b = mb.addButton(btn) + if not default: + default = b + mb.setDefaultButton(default) + else: + b = mb.addButton(QMessageBox.Ok) + b.setDefault(True) if help: b = mb.addButton(QMessageBox.Help) b.clicked.connect(lambda: openHelp(help)) From e3b7096db520f85a1f9cbd6125cc414b46b30cd7 Mon Sep 17 00:00:00 2001 From: Glutanimate Date: Fri, 3 Jan 2020 17:57:33 +0100 Subject: [PATCH 3/7] Extend CLI with the ability to install .ankiaddon packages Allows Anki to register a mime-type handler for .ankiaddon files Other small collateral changes: + fix positioning issues with some prompts and progress dialog + add prompt titles where they were missing + add type annotations for AddonManager installation methods + explicitly import os in main (used to be imported via aqt.qt) --- qt/aqt/__init__.py | 2 +- qt/aqt/addons.py | 89 +++++++++++++++++++++++++++++++++++++--------- qt/aqt/main.py | 25 +++++++++++-- 3 files changed, 96 insertions(+), 20 deletions(-) diff --git a/qt/aqt/__init__.py b/qt/aqt/__init__.py index 83d69e3a3..1400a6b71 100644 --- a/qt/aqt/__init__.py +++ b/qt/aqt/__init__.py @@ -261,7 +261,7 @@ def parseArgs(argv): if isMac and len(argv) > 1 and argv[1].startswith("-psn"): argv = [argv[0]] parser = argparse.ArgumentParser(description="Anki " + appVersion) - parser.usage = "%(prog)s [OPTIONS] [file to import]" + parser.usage = "%(prog)s [OPTIONS] [file to import/add-on to install]" parser.add_argument("-b", "--base", help="path to base folder", default="") parser.add_argument("-p", "--profile", help="profile name to load", default="") parser.add_argument("-l", "--lang", help="interface language (en, de, etc)") diff --git a/qt/aqt/addons.py b/qt/aqt/addons.py index 31f74f252..23a42a00d 100644 --- a/qt/aqt/addons.py +++ b/qt/aqt/addons.py @@ -7,7 +7,7 @@ import os import re import zipfile from collections import defaultdict -from typing import Any, Callable, Dict, List, NamedTuple, Optional, Tuple +from typing import IO, Any, Callable, Dict, List, NamedTuple, Optional, Tuple, Union from zipfile import ZipFile import jsonschema @@ -209,7 +209,9 @@ and have been disabled: %(found)s" return {} return manifest - def install(self, file, manifest=None) -> AddonInstallationResult: + def install( + self, file: Union[IO, str], manifest: dict = None + ) -> AddonInstallationResult: """Install add-on from path or file-like object. Metadata is read from the manifest file, with keys overriden by supplying a 'manifest' dictionary""" @@ -284,11 +286,14 @@ and have been disabled: %(found)s" # Processing local add-on files ###################################################################### - def processPackages(self, paths) -> Tuple[List[str], List[str]]: + def processPackages( + self, paths: List[str], parent: QWidget = None + ) -> Tuple[List[str], List[str]]: + log = [] errs = [] - self.mw.progress.start(immediate=True) + self.mw.progress.start(immediate=True, parent=parent) try: for path in paths: base = os.path.basename(path) @@ -661,7 +666,7 @@ class AddonsDialog(QDialog): def onGetAddons(self): GetAddons(self) - def onInstallFiles(self, paths=None): + def onInstallFiles(self, paths: List[str] = None, external: bool = False): if not paths: key = _("Packaged Anki Add-on") + " (*{})".format(self.mgr.ext) paths = getFile( @@ -670,17 +675,7 @@ class AddonsDialog(QDialog): if not paths: return False - log, errs = self.mgr.processPackages(paths) - - if log: - log_html = "
".join(log) - if len(log) == 1: - tooltip(log_html, parent=self) - else: - showInfo(log_html, parent=self, textFormat="rich") - if errs: - msg = _("Please report this to the respective add-on author(s).") - showWarning("

".join(errs + [msg]), parent=self, textFormat="rich") + installAddonPackages(self.mgr, paths, parent=self) self.redrawAddons() @@ -855,3 +850,65 @@ class ConfigEditor(QDialog): self.onClose() super().accept() + + +# .ankiaddon installation wizard +###################################################################### + + +def installAddonPackages( + addonsManager: AddonManager, + paths: List[str], + parent: QWidget = None, + external: bool = False, +) -> bool: + + if external: + names_str = ",
".join(f"{os.path.basename(p)}" for p in paths) + q = _( + "Important: As add-ons are programs downloaded from the internet, " + "they are potentially malicious." + "You should only install add-ons you trust.

" + "Are you sure you want to proceed with the installation of the " + f"following add-on(s)?

{names_str}" + ) + if ( + not showInfo( + q, + parent=parent, + title=_("Install Anki add-on"), + type="warning", + customBtns=[QMessageBox.No, QMessageBox.Yes], + ) + == QMessageBox.Yes + ): + tooltip(_("Add-on installation aborted"), parent=parent) + return False + + log, errs = addonsManager.processPackages(paths, parent=parent) + + if log: + log_html = "
".join(log) + if external: + log_html += "

" + _( + "Please restart Anki to complete the installation." + ) + if len(log) == 1: + tooltip(log_html, parent=parent) + else: + showInfo( + log_html, + parent=parent, + textFormat="rich", + title=_("Installation complete"), + ) + if errs: + msg = _("Please report this to the respective add-on author(s).") + showWarning( + "

".join(errs + [msg]), + parent=parent, + textFormat="rich", + title=_("Add-on installation error"), + ) + + return not errs diff --git a/qt/aqt/main.py b/qt/aqt/main.py index 2ae1039e7..d759cf34a 100644 --- a/qt/aqt/main.py +++ b/qt/aqt/main.py @@ -4,6 +4,7 @@ import faulthandler import gc +import os import platform import re import signal @@ -1021,6 +1022,13 @@ QTreeWidget { aqt.exporting.ExportDialog(self, did=did) + # Installing add-ons from CLI / mimetype handler + ########################################################################## + + def installAddon(self, path): + from aqt.addons import installAddonPackages + installAddonPackages(self.addonManager, [path], external=True, parent=self) + # Cramming ########################################################################## @@ -1473,6 +1481,8 @@ will be lost. Continue?""" self.app.appMsg.connect(self.onAppMsg) def onAppMsg(self, buf: str) -> Optional[QTimer]: + is_addon = buf.endswith(".ankiaddon") + if self.state == "startup": # try again in a second return self.progress.timer( @@ -1483,7 +1493,11 @@ will be lost. Continue?""" if buf == "raise": return None self.pendingImport = buf - return tooltip(_("Deck will be imported when a profile is opened.")) + if is_addon: + msg = _("Add-on will be installed when a profile is opened.") + else: + msg = _("Deck will be imported when a profile is opened.") + return tooltip(msg) if not self.interactiveState() or self.progress.busy(): # we can't raise the main window while in profile dialog, syncing, etc if buf != "raise": @@ -1507,8 +1521,13 @@ Please ensure a profile is open and Anki is not busy, then try again.""" self.raise_() if buf == "raise": return None - # import - self.handleImport(buf) + + # import / add-on installation + if is_addon: + self.installAddon(buf) + else: + self.handleImport(buf) + return None # GC From 57c48d7c85e1aa0041d9fb385c7cc4f400a671e1 Mon Sep 17 00:00:00 2001 From: Glutanimate Date: Fri, 3 Jan 2020 17:58:11 +0100 Subject: [PATCH 4/7] Add .ankiaddon mime-type on Linux and register Anki as its handler --- qt/anki.desktop | 2 +- qt/anki.xml | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/qt/anki.desktop b/qt/anki.desktop index fb4ac4f83..374a4b5df 100644 --- a/qt/anki.desktop +++ b/qt/anki.desktop @@ -9,4 +9,4 @@ Categories=Education;Languages;KDE;Qt; Terminal=false Type=Application Version=1.0 -MimeType=application/x-apkg;application/x-anki; +MimeType=application/x-apkg;application/x-anki;application/x-ankiaddon; diff --git a/qt/anki.xml b/qt/anki.xml index 09fdc9c69..5bfd2ca0f 100644 --- a/qt/anki.xml +++ b/qt/anki.xml @@ -11,4 +11,9 @@ + + Anki 2.1 add-on package + + + From 1b236acb3dabd9c1157676057f4dfcbafc59b278 Mon Sep 17 00:00:00 2001 From: Glutanimate Date: Fri, 3 Jan 2020 18:23:28 +0100 Subject: [PATCH 5/7] Fix mypy and black checks --- qt/aqt/addons.py | 4 ++-- qt/aqt/main.py | 7 ++++--- qt/aqt/utils.py | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/qt/aqt/addons.py b/qt/aqt/addons.py index 23a42a00d..29093ff90 100644 --- a/qt/aqt/addons.py +++ b/qt/aqt/addons.py @@ -47,8 +47,8 @@ class AddonInstallationResult(NamedTuple): class AddonManager: - ext = ".ankiaddon" - _manifest_schema = { + ext: str = ".ankiaddon" + _manifest_schema: dict = { "type": "object", "properties": { "package": {"type": "string", "meta": False}, diff --git a/qt/aqt/main.py b/qt/aqt/main.py index d759cf34a..2cc6cf7ad 100644 --- a/qt/aqt/main.py +++ b/qt/aqt/main.py @@ -1027,6 +1027,7 @@ QTreeWidget { def installAddon(self, path): from aqt.addons import installAddonPackages + installAddonPackages(self.addonManager, [path], external=True, parent=self) # Cramming @@ -1482,7 +1483,7 @@ will be lost. Continue?""" def onAppMsg(self, buf: str) -> Optional[QTimer]: is_addon = buf.endswith(".ankiaddon") - + if self.state == "startup": # try again in a second return self.progress.timer( @@ -1521,13 +1522,13 @@ Please ensure a profile is open and Anki is not busy, then try again.""" self.raise_() if buf == "raise": return None - + # import / add-on installation if is_addon: self.installAddon(buf) else: self.handleImport(buf) - + return None # GC diff --git a/qt/aqt/utils.py b/qt/aqt/utils.py index c32c409dc..6b22c8f27 100644 --- a/qt/aqt/utils.py +++ b/qt/aqt/utils.py @@ -57,7 +57,7 @@ def showInfo( type="info", title="Anki", textFormat=None, - customBtns=None + customBtns=None, ): "Show a small info window with an OK button." if parent is False: From 14e3c24e734829dea119c86b09057b7bfde01f6b Mon Sep 17 00:00:00 2001 From: Yann Salimi Date: Fri, 3 Jan 2020 18:37:50 +0100 Subject: [PATCH 6/7] Add AMBOSS as a contributor --- CONTRIBUTORS | 1 + qt/aqt/about.py | 1 + 2 files changed, 2 insertions(+) diff --git a/CONTRIBUTORS b/CONTRIBUTORS index f41d0d36f..094492b7f 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -19,6 +19,7 @@ support site, it would be great if you could add your name below as well. ******************** Erez Volk +AMBOSS MD Inc. ******************** diff --git a/qt/aqt/about.py b/qt/aqt/about.py index 41e180725..53952e311 100644 --- a/qt/aqt/about.py +++ b/qt/aqt/about.py @@ -149,6 +149,7 @@ system. It's free and open source." "Arman High", "Arthur Milchior", "Rai (Michael Pokorny)", + "AMBOSS MD Inc.", ) ) From 392938f20c3bf835b21569373aad1fb0b0429853 Mon Sep 17 00:00:00 2001 From: Glutanimate Date: Sat, 4 Jan 2020 01:30:20 +0100 Subject: [PATCH 7/7] Adjust type annotations, format string, and remove tooltip on cancel Other areas of Anki don't confirm cancelling actions with tooltips, so after further consideration, the tooltip felt out of the place and might have actually confused users into thinking some action was being performed. --- qt/aqt/addons.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/qt/aqt/addons.py b/qt/aqt/addons.py index 29093ff90..1bac9266b 100644 --- a/qt/aqt/addons.py +++ b/qt/aqt/addons.py @@ -666,7 +666,7 @@ class AddonsDialog(QDialog): def onGetAddons(self): GetAddons(self) - def onInstallFiles(self, paths: List[str] = None, external: bool = False): + def onInstallFiles(self, paths: Optional[List[str]] = None, external: bool = False): if not paths: key = _("Packaged Anki Add-on") + " (*{})".format(self.mgr.ext) paths = getFile( @@ -864,14 +864,14 @@ def installAddonPackages( ) -> bool: if external: - names_str = ",
".join(f"{os.path.basename(p)}" for p in paths) + names = ",
".join(f"{os.path.basename(p)}" for p in paths) q = _( "Important: As add-ons are programs downloaded from the internet, " "they are potentially malicious." "You should only install add-ons you trust.

" "Are you sure you want to proceed with the installation of the " - f"following add-on(s)?

{names_str}" - ) + "following add-on(s)?

%(names)s" + ) % dict(names=names) if ( not showInfo( q, @@ -882,7 +882,6 @@ def installAddonPackages( ) == QMessageBox.Yes ): - tooltip(_("Add-on installation aborted"), parent=parent) return False log, errs = addonsManager.processPackages(paths, parent=parent)