From f1488b5983e1a1ff9d5a71dc9172e35a339f36d2 Mon Sep 17 00:00:00 2001 From: RumovZ Date: Mon, 28 Mar 2022 14:17:50 +0200 Subject: [PATCH] Card type error (#1749) * TemplateSaveError -> CardTypeError * Don't show success tooltip if export fails * Attach help page to error Show help link if export fails due to card type error. * Add type (dae) * Add shared show_exception() (dae) - Use a shared routine for printing standard backend errors, so that we can take advantage of the help links in eg. the card layout screen as well. - The truthiness check on help in showInfo() would have ignored the enum 0 value. - Close the exporting dialog on a documented failure as well * Fix local variable help_page --- proto/anki/backend.proto | 5 ++++ proto/anki/links.proto | 5 ++++ pylib/anki/_backend/__init__.py | 6 ++++- pylib/anki/errors.py | 16 +++++++++++++ qt/aqt/errors.py | 31 ++++++++++++++++++++---- qt/aqt/exporting.py | 15 ++++++++---- qt/aqt/operations/__init__.py | 3 ++- qt/aqt/utils.py | 8 +++---- rslib/src/backend/error.rs | 4 +++- rslib/src/error/mod.rs | 42 +++++++++++++++++++++------------ rslib/src/links.rs | 11 +++++++++ rslib/src/notetype/mod.rs | 18 +++++++------- 12 files changed, 125 insertions(+), 39 deletions(-) diff --git a/proto/anki/backend.proto b/proto/anki/backend.proto index b5dd17460..e24c63992 100644 --- a/proto/anki/backend.proto +++ b/proto/anki/backend.proto @@ -5,6 +5,8 @@ syntax = "proto3"; package anki.backend; +import "anki/links.proto"; + /// while the protobuf descriptors expose the order services are defined in, /// that information is not available in prost, so we define an enum to make /// sure all clients agree on the service index @@ -59,10 +61,13 @@ message BackendError { CUSTOM_STUDY_ERROR = 15; IMPORT_ERROR = 16; DELETED = 17; + CARD_TYPE_ERROR = 18; } // localized error description suitable for displaying to the user string localized = 1; // the error subtype Kind kind = 2; + // optional page in the manual + optional links.HelpPageLinkRequest.HelpPage help_page = 3; } diff --git a/proto/anki/links.proto b/proto/anki/links.proto index f74bdea21..7566f91ee 100644 --- a/proto/anki/links.proto +++ b/proto/anki/links.proto @@ -31,6 +31,11 @@ message HelpPageLinkRequest { DECK_OPTIONS = 15; EDITING_FEATURES = 16; FULL_SCREEN_ISSUE = 17; + CARD_TYPE_DUPLICATE = 18; + CARD_TYPE_NO_FRONT_FIELD = 19; + CARD_TYPE_MISSING_CLOZE = 20; + CARD_TYPE_EXTRANEOUS_CLOZE = 21; + CARD_TYPE_TEMPLATE_ERROR = 22; } HelpPage page = 1; } diff --git a/pylib/anki/_backend/__init__.py b/pylib/anki/_backend/__init__.py index e0084a326..48eeac94f 100644 --- a/pylib/anki/_backend/__init__.py +++ b/pylib/anki/_backend/__init__.py @@ -5,7 +5,7 @@ from __future__ import annotations import sys import traceback -from typing import Any, Sequence, Union +from typing import Any, Sequence from weakref import ref from markdown import markdown @@ -20,6 +20,7 @@ from anki.utils import from_json_bytes, to_json_bytes from ..errors import ( BackendIOError, + CardTypeError, CustomStudyError, DBError, ExistsError, @@ -189,6 +190,9 @@ def backend_exception_to_pylib(err: backend_pb2.BackendError) -> Exception: elif val == kind.DB_ERROR: return DBError(err.localized) + elif val == kind.CARD_TYPE_ERROR: + return CardTypeError(err.localized, err.help_page) + elif val == kind.TEMPLATE_PARSE: return TemplateError(err.localized) diff --git a/pylib/anki/errors.py b/pylib/anki/errors.py index a1aa3d84d..35fb6d013 100644 --- a/pylib/anki/errors.py +++ b/pylib/anki/errors.py @@ -4,6 +4,10 @@ from __future__ import annotations from enum import Enum +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + import anki.collection class LocalizedError(Exception): @@ -17,6 +21,14 @@ class LocalizedError(Exception): return self._localized +class DocumentedError(LocalizedError): + """A localized error described in the manual.""" + + def __init__(self, localized: str, help_page: anki.collection.HelpPage.V) -> None: + self.help_page = help_page + super().__init__(localized) + + class Interrupted(Exception): pass @@ -48,6 +60,10 @@ class DBError(LocalizedError): pass +class CardTypeError(DocumentedError): + pass + + class TemplateError(LocalizedError): pass diff --git a/qt/aqt/errors.py b/qt/aqt/errors.py index c517a2c42..618549e1a 100644 --- a/qt/aqt/errors.py +++ b/qt/aqt/errors.py @@ -1,18 +1,41 @@ # Copyright: Ankitects Pty Ltd and contributors # License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html + +from __future__ import annotations + import html import re import sys import traceback -from typing import Optional, TextIO, cast +from typing import TYPE_CHECKING, Optional, TextIO, cast from markdown import markdown -from aqt import mw -from aqt.main import AnkiQt +import aqt +from anki.errors import DocumentedError, LocalizedError from aqt.qt import * from aqt.utils import showText, showWarning, supportText, tr +if TYPE_CHECKING: + from aqt.main import AnkiQt + + +def show_exception(*, parent: QWidget, exception: Exception) -> None: + "Present a caught exception to the user using a pop-up." + if isinstance(exception, InterruptedError): + # nothing to do + return + help_page = exception.help_page if isinstance(exception, DocumentedError) else None + if not isinstance(exception, LocalizedError): + # if the error is not originating from the backend, dump + # a traceback to the console to aid in debugging + traceback.print_exception( + None, exception, exception.__traceback__, file=sys.stdout + ) + + showWarning(str(exception), parent=parent, help=help_page) + + if not os.environ.get("DEBUG"): def excepthook(etype, val, tb) -> None: # type: ignore @@ -121,7 +144,7 @@ class ErrorHandler(QObject): return "" # reverse to list most likely suspect first, dict to deduplicate: addons = [ - mw.addonManager.addonName(i) for i in dict.fromkeys(reversed(matches)) + aqt.mw.addonManager.addonName(i) for i in dict.fromkeys(reversed(matches)) ] # highlight importance of first add-on: addons[0] = f"{addons[0]}" diff --git a/qt/aqt/exporting.py b/qt/aqt/exporting.py index 4866b6926..de8009760 100644 --- a/qt/aqt/exporting.py +++ b/qt/aqt/exporting.py @@ -15,6 +15,7 @@ from anki import hooks from anki.cards import CardId from anki.decks import DeckId from anki.exporting import Exporter, exporters +from aqt.errors import show_exception from aqt.qt import * from aqt.utils import ( checkInvalidFilename, @@ -174,10 +175,11 @@ class ExportDialog(QDialog): try: # raises if exporter failed future.result() - except Exception as e: - traceback.print_exc(file=sys.stdout) - showWarning(str(e)) - self.on_export_finished() + except Exception as exc: + show_exception(parent=self.mw, exception=exc) + self.on_export_failed() + else: + self.on_export_finished() self.mw.progress.start() hooks.media_files_did_export.append(exported_media) @@ -195,3 +197,8 @@ class ExportDialog(QDialog): msg = tr.exporting_card_exported(count=self.exporter.count) tooltip(msg, period=3000) QDialog.reject(self) + + def on_export_failed(self) -> None: + if self.isVerbatim: + self.mw.reopen() + QDialog.reject(self) diff --git a/qt/aqt/operations/__init__.py b/qt/aqt/operations/__init__.py index a2d3eb7db..0fd4dbad1 100644 --- a/qt/aqt/operations/__init__.py +++ b/qt/aqt/operations/__init__.py @@ -15,6 +15,7 @@ from anki.collection import ( OpChangesWithCount, OpChangesWithId, ) +from aqt.errors import show_exception from aqt.qt import QWidget from aqt.utils import showWarning @@ -101,7 +102,7 @@ class CollectionOp(Generic[ResultWithChanges]): if self._failure: self._failure(exception) else: - showWarning(str(exception), self._parent) + show_exception(parent=self._parent, exception=exception) return else: # BaseException like SystemExit; rethrow it diff --git a/qt/aqt/utils.py b/qt/aqt/utils.py index 52a270970..3c4ed09b3 100644 --- a/qt/aqt/utils.py +++ b/qt/aqt/utils.py @@ -28,7 +28,7 @@ from aqt.qt import * from aqt.theme import theme_manager if TYPE_CHECKING: - TextFormat = Union[Literal["plain", "rich"]] + TextFormat = Literal["plain", "rich"] def aqt_data_folder() -> str: @@ -73,7 +73,7 @@ def openLink(link: str | QUrl) -> None: def showWarning( text: str, parent: QWidget | None = None, - help: HelpPageArgument = "", + help: HelpPageArgument | None = None, title: str = "Anki", textFormat: TextFormat | None = None, ) -> int: @@ -95,7 +95,7 @@ def showCritical( def showInfo( text: str, parent: QWidget | None = None, - help: HelpPageArgument = "", + help: HelpPageArgument | None = None, type: str = "info", title: str = "Anki", textFormat: TextFormat | None = None, @@ -133,7 +133,7 @@ def showInfo( else: b = mb.addButton(QMessageBox.StandardButton.Ok) b.setDefault(True) - if help: + if help is not None: b = mb.addButton(QMessageBox.StandardButton.Help) qconnect(b.clicked, lambda: openHelp(help)) b.setAutoDefault(False) diff --git a/rslib/src/backend/error.rs b/rslib/src/backend/error.rs index 5ff6e14a1..85ed1ed90 100644 --- a/rslib/src/backend/error.rs +++ b/rslib/src/backend/error.rs @@ -11,6 +11,7 @@ use crate::{ impl AnkiError { pub(super) fn into_protobuf(self, tr: &I18n) -> pb::BackendError { let localized = self.localized_description(tr); + let help_page = self.help_page().map(|page| page as i32); let kind = match self { AnkiError::InvalidInput(_) => Kind::InvalidInput, AnkiError::TemplateError(_) => Kind::TemplateParse, @@ -28,7 +29,7 @@ impl AnkiError { AnkiError::Existing => Kind::Exists, AnkiError::FilteredDeckError(_) => Kind::FilteredDeckError, AnkiError::SearchError(_) => Kind::SearchError, - AnkiError::TemplateSaveError(_) => Kind::TemplateParse, + AnkiError::CardTypeError(_) => Kind::CardTypeError, AnkiError::ParseNumError => Kind::InvalidInput, AnkiError::InvalidRegex(_) => Kind::InvalidInput, AnkiError::UndoEmpty => Kind::UndoEmpty, @@ -43,6 +44,7 @@ impl AnkiError { pb::BackendError { kind: kind as i32, localized, + help_page, } } } diff --git a/rslib/src/error/mod.rs b/rslib/src/error/mod.rs index 1f078208c..4c1eae0a7 100644 --- a/rslib/src/error/mod.rs +++ b/rslib/src/error/mod.rs @@ -14,7 +14,7 @@ pub use network::{NetworkError, NetworkErrorKind, SyncError, SyncErrorKind}; pub use search::{ParseError, SearchErrorKind}; use tempfile::PathPersistError; -use crate::i18n::I18n; +use crate::{i18n::I18n, links::HelpPage}; pub type Result = std::result::Result; @@ -22,7 +22,7 @@ pub type Result = std::result::Result; pub enum AnkiError { InvalidInput(String), TemplateError(String), - TemplateSaveError(TemplateSaveError), + CardTypeError(CardTypeError), IoError(String), FileIoError(FileIoError), DbError(DbError), @@ -71,20 +71,17 @@ impl AnkiError { // already localized info.into() } - AnkiError::TemplateSaveError(err) => { + AnkiError::CardTypeError(err) => { let header = tr.card_templates_invalid_template_number(err.ordinal + 1, &err.notetype); let details = match err.details { - TemplateSaveErrorDetails::TemplateError - | TemplateSaveErrorDetails::NoSuchField => tr.card_templates_see_preview(), - TemplateSaveErrorDetails::NoFrontField => tr.card_templates_no_front_field(), - TemplateSaveErrorDetails::Duplicate(i) => { - tr.card_templates_identical_front(i + 1) - } - TemplateSaveErrorDetails::MissingCloze => tr.card_templates_missing_cloze(), - TemplateSaveErrorDetails::ExtraneousCloze => { - tr.card_templates_extraneous_cloze() + CardTypeErrorDetails::TemplateError | CardTypeErrorDetails::NoSuchField => { + tr.card_templates_see_preview() } + CardTypeErrorDetails::NoFrontField => tr.card_templates_no_front_field(), + CardTypeErrorDetails::Duplicate(i) => tr.card_templates_identical_front(i + 1), + CardTypeErrorDetails::MissingCloze => tr.card_templates_missing_cloze(), + CardTypeErrorDetails::ExtraneousCloze => tr.card_templates_extraneous_cloze(), }; format!("{}
{}", header, details) } @@ -120,6 +117,21 @@ impl AnkiError { } } } + + pub fn help_page(&self) -> Option { + match self { + Self::CardTypeError(CardTypeError { details, .. }) => Some(match details { + CardTypeErrorDetails::TemplateError | CardTypeErrorDetails::NoSuchField => { + HelpPage::CardTypeTemplateError + } + CardTypeErrorDetails::Duplicate(_) => HelpPage::CardTypeDuplicate, + CardTypeErrorDetails::NoFrontField => HelpPage::CardTypeNoFrontField, + CardTypeErrorDetails::MissingCloze => HelpPage::CardTypeMissingCloze, + CardTypeErrorDetails::ExtraneousCloze => HelpPage::CardTypeExtraneousCloze, + }), + _ => None, + } + } } #[derive(Debug, PartialEq)] @@ -174,14 +186,14 @@ impl From for AnkiError { } #[derive(Debug, PartialEq)] -pub struct TemplateSaveError { +pub struct CardTypeError { pub notetype: String, pub ordinal: usize, - pub details: TemplateSaveErrorDetails, + pub details: CardTypeErrorDetails, } #[derive(Debug, PartialEq)] -pub enum TemplateSaveErrorDetails { +pub enum CardTypeErrorDetails { TemplateError, Duplicate(usize), NoFrontField, diff --git a/rslib/src/links.rs b/rslib/src/links.rs index 739bc2ee4..670dc0178 100644 --- a/rslib/src/links.rs +++ b/rslib/src/links.rs @@ -30,6 +30,17 @@ impl HelpPage { HelpPage::DeckOptions => "deck-options.html", HelpPage::EditingFeatures => "editing.html#editing-features", HelpPage::FullScreenIssue => "platform/windows/display-issues.html#full-screen", + HelpPage::CardTypeTemplateError => "templates/errors.html#template-syntax-error", + HelpPage::CardTypeDuplicate => "templates/errors.html#identical-front-sides", + HelpPage::CardTypeNoFrontField => { + "templates/errors.html#no-field-replacement-on-front-side" + } + HelpPage::CardTypeMissingCloze => { + "templates/errors.html#no-cloze-filter-on-cloze-notetype" + } + HelpPage::CardTypeExtraneousCloze => { + "templates/errors.html#cloze-filter-outside-cloze-notetype" + } } } } diff --git a/rslib/src/notetype/mod.rs b/rslib/src/notetype/mod.rs index 233722f18..eabf38b98 100644 --- a/rslib/src/notetype/mod.rs +++ b/rslib/src/notetype/mod.rs @@ -42,7 +42,7 @@ pub use crate::backend_proto::{ }; use crate::{ define_newtype, - error::{TemplateSaveError, TemplateSaveErrorDetails}, + error::{CardTypeError, CardTypeErrorDetails}, prelude::*, search::{Node, SearchNode}, storage::comma_separated_ids, @@ -341,10 +341,10 @@ impl Notetype { for (index, card) in self.templates.iter().enumerate() { if let Some(old_index) = map.insert(&card.config.q_format, index) { if !CARD_TAG.is_match(&card.config.q_format) { - return Err(AnkiError::TemplateSaveError(TemplateSaveError { + return Err(AnkiError::CardTypeError(CardTypeError { notetype: self.name.clone(), ordinal: index, - details: TemplateSaveErrorDetails::Duplicate(old_index), + details: CardTypeErrorDetails::Duplicate(old_index), })); } } @@ -364,18 +364,18 @@ impl Notetype { if let (Some(q), Some(a)) = sides { let q_fields = q.fields(); if q_fields.is_empty() { - Some((index, TemplateSaveErrorDetails::NoFrontField)) + Some((index, CardTypeErrorDetails::NoFrontField)) } else if self.unknown_field_name(q_fields.union(&a.fields())) { - Some((index, TemplateSaveErrorDetails::NoSuchField)) + Some((index, CardTypeErrorDetails::NoSuchField)) } else { None } } else { - Some((index, TemplateSaveErrorDetails::TemplateError)) + Some((index, CardTypeErrorDetails::TemplateError)) } }) { - Err(AnkiError::TemplateSaveError(TemplateSaveError { + Err(AnkiError::CardTypeError(CardTypeError { notetype: self.name.clone(), ordinal: invalid_index, details, @@ -405,10 +405,10 @@ impl Notetype { parsed_templates: &[(Option, Option)], ) -> Result<()> { if self.is_cloze() && missing_cloze_filter(parsed_templates) { - return Err(AnkiError::TemplateSaveError(TemplateSaveError { + return Err(AnkiError::CardTypeError(CardTypeError { notetype: self.name.clone(), ordinal: 0, - details: TemplateSaveErrorDetails::MissingCloze, + details: CardTypeErrorDetails::MissingCloze, })); } Ok(())