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
This commit is contained in:
RumovZ 2022-03-28 14:17:50 +02:00 committed by GitHub
parent dd16890c11
commit f1488b5983
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 125 additions and 39 deletions

View file

@ -5,6 +5,8 @@ syntax = "proto3";
package anki.backend; package anki.backend;
import "anki/links.proto";
/// while the protobuf descriptors expose the order services are defined in, /// 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 /// that information is not available in prost, so we define an enum to make
/// sure all clients agree on the service index /// sure all clients agree on the service index
@ -59,10 +61,13 @@ message BackendError {
CUSTOM_STUDY_ERROR = 15; CUSTOM_STUDY_ERROR = 15;
IMPORT_ERROR = 16; IMPORT_ERROR = 16;
DELETED = 17; DELETED = 17;
CARD_TYPE_ERROR = 18;
} }
// localized error description suitable for displaying to the user // localized error description suitable for displaying to the user
string localized = 1; string localized = 1;
// the error subtype // the error subtype
Kind kind = 2; Kind kind = 2;
// optional page in the manual
optional links.HelpPageLinkRequest.HelpPage help_page = 3;
} }

View file

@ -31,6 +31,11 @@ message HelpPageLinkRequest {
DECK_OPTIONS = 15; DECK_OPTIONS = 15;
EDITING_FEATURES = 16; EDITING_FEATURES = 16;
FULL_SCREEN_ISSUE = 17; 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; HelpPage page = 1;
} }

View file

@ -5,7 +5,7 @@ from __future__ import annotations
import sys import sys
import traceback import traceback
from typing import Any, Sequence, Union from typing import Any, Sequence
from weakref import ref from weakref import ref
from markdown import markdown from markdown import markdown
@ -20,6 +20,7 @@ from anki.utils import from_json_bytes, to_json_bytes
from ..errors import ( from ..errors import (
BackendIOError, BackendIOError,
CardTypeError,
CustomStudyError, CustomStudyError,
DBError, DBError,
ExistsError, ExistsError,
@ -189,6 +190,9 @@ def backend_exception_to_pylib(err: backend_pb2.BackendError) -> Exception:
elif val == kind.DB_ERROR: elif val == kind.DB_ERROR:
return DBError(err.localized) return DBError(err.localized)
elif val == kind.CARD_TYPE_ERROR:
return CardTypeError(err.localized, err.help_page)
elif val == kind.TEMPLATE_PARSE: elif val == kind.TEMPLATE_PARSE:
return TemplateError(err.localized) return TemplateError(err.localized)

View file

@ -4,6 +4,10 @@
from __future__ import annotations from __future__ import annotations
from enum import Enum from enum import Enum
from typing import TYPE_CHECKING
if TYPE_CHECKING:
import anki.collection
class LocalizedError(Exception): class LocalizedError(Exception):
@ -17,6 +21,14 @@ class LocalizedError(Exception):
return self._localized 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): class Interrupted(Exception):
pass pass
@ -48,6 +60,10 @@ class DBError(LocalizedError):
pass pass
class CardTypeError(DocumentedError):
pass
class TemplateError(LocalizedError): class TemplateError(LocalizedError):
pass pass

View file

@ -1,18 +1,41 @@
# Copyright: Ankitects Pty Ltd and contributors # Copyright: Ankitects Pty Ltd and contributors
# License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html # License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
from __future__ import annotations
import html import html
import re import re
import sys import sys
import traceback import traceback
from typing import Optional, TextIO, cast from typing import TYPE_CHECKING, Optional, TextIO, cast
from markdown import markdown from markdown import markdown
from aqt import mw import aqt
from aqt.main import AnkiQt from anki.errors import DocumentedError, LocalizedError
from aqt.qt import * from aqt.qt import *
from aqt.utils import showText, showWarning, supportText, tr 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"): if not os.environ.get("DEBUG"):
def excepthook(etype, val, tb) -> None: # type: ignore def excepthook(etype, val, tb) -> None: # type: ignore
@ -121,7 +144,7 @@ class ErrorHandler(QObject):
return "" return ""
# reverse to list most likely suspect first, dict to deduplicate: # reverse to list most likely suspect first, dict to deduplicate:
addons = [ 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: # highlight importance of first add-on:
addons[0] = f"<b>{addons[0]}</b>" addons[0] = f"<b>{addons[0]}</b>"

View file

@ -15,6 +15,7 @@ from anki import hooks
from anki.cards import CardId from anki.cards import CardId
from anki.decks import DeckId from anki.decks import DeckId
from anki.exporting import Exporter, exporters from anki.exporting import Exporter, exporters
from aqt.errors import show_exception
from aqt.qt import * from aqt.qt import *
from aqt.utils import ( from aqt.utils import (
checkInvalidFilename, checkInvalidFilename,
@ -174,10 +175,11 @@ class ExportDialog(QDialog):
try: try:
# raises if exporter failed # raises if exporter failed
future.result() future.result()
except Exception as e: except Exception as exc:
traceback.print_exc(file=sys.stdout) show_exception(parent=self.mw, exception=exc)
showWarning(str(e)) self.on_export_failed()
self.on_export_finished() else:
self.on_export_finished()
self.mw.progress.start() self.mw.progress.start()
hooks.media_files_did_export.append(exported_media) hooks.media_files_did_export.append(exported_media)
@ -195,3 +197,8 @@ class ExportDialog(QDialog):
msg = tr.exporting_card_exported(count=self.exporter.count) msg = tr.exporting_card_exported(count=self.exporter.count)
tooltip(msg, period=3000) tooltip(msg, period=3000)
QDialog.reject(self) QDialog.reject(self)
def on_export_failed(self) -> None:
if self.isVerbatim:
self.mw.reopen()
QDialog.reject(self)

View file

@ -15,6 +15,7 @@ from anki.collection import (
OpChangesWithCount, OpChangesWithCount,
OpChangesWithId, OpChangesWithId,
) )
from aqt.errors import show_exception
from aqt.qt import QWidget from aqt.qt import QWidget
from aqt.utils import showWarning from aqt.utils import showWarning
@ -101,7 +102,7 @@ class CollectionOp(Generic[ResultWithChanges]):
if self._failure: if self._failure:
self._failure(exception) self._failure(exception)
else: else:
showWarning(str(exception), self._parent) show_exception(parent=self._parent, exception=exception)
return return
else: else:
# BaseException like SystemExit; rethrow it # BaseException like SystemExit; rethrow it

View file

@ -28,7 +28,7 @@ from aqt.qt import *
from aqt.theme import theme_manager from aqt.theme import theme_manager
if TYPE_CHECKING: if TYPE_CHECKING:
TextFormat = Union[Literal["plain", "rich"]] TextFormat = Literal["plain", "rich"]
def aqt_data_folder() -> str: def aqt_data_folder() -> str:
@ -73,7 +73,7 @@ def openLink(link: str | QUrl) -> None:
def showWarning( def showWarning(
text: str, text: str,
parent: QWidget | None = None, parent: QWidget | None = None,
help: HelpPageArgument = "", help: HelpPageArgument | None = None,
title: str = "Anki", title: str = "Anki",
textFormat: TextFormat | None = None, textFormat: TextFormat | None = None,
) -> int: ) -> int:
@ -95,7 +95,7 @@ def showCritical(
def showInfo( def showInfo(
text: str, text: str,
parent: QWidget | None = None, parent: QWidget | None = None,
help: HelpPageArgument = "", help: HelpPageArgument | None = None,
type: str = "info", type: str = "info",
title: str = "Anki", title: str = "Anki",
textFormat: TextFormat | None = None, textFormat: TextFormat | None = None,
@ -133,7 +133,7 @@ def showInfo(
else: else:
b = mb.addButton(QMessageBox.StandardButton.Ok) b = mb.addButton(QMessageBox.StandardButton.Ok)
b.setDefault(True) b.setDefault(True)
if help: if help is not None:
b = mb.addButton(QMessageBox.StandardButton.Help) b = mb.addButton(QMessageBox.StandardButton.Help)
qconnect(b.clicked, lambda: openHelp(help)) qconnect(b.clicked, lambda: openHelp(help))
b.setAutoDefault(False) b.setAutoDefault(False)

View file

@ -11,6 +11,7 @@ use crate::{
impl AnkiError { impl AnkiError {
pub(super) fn into_protobuf(self, tr: &I18n) -> pb::BackendError { pub(super) fn into_protobuf(self, tr: &I18n) -> pb::BackendError {
let localized = self.localized_description(tr); let localized = self.localized_description(tr);
let help_page = self.help_page().map(|page| page as i32);
let kind = match self { let kind = match self {
AnkiError::InvalidInput(_) => Kind::InvalidInput, AnkiError::InvalidInput(_) => Kind::InvalidInput,
AnkiError::TemplateError(_) => Kind::TemplateParse, AnkiError::TemplateError(_) => Kind::TemplateParse,
@ -28,7 +29,7 @@ impl AnkiError {
AnkiError::Existing => Kind::Exists, AnkiError::Existing => Kind::Exists,
AnkiError::FilteredDeckError(_) => Kind::FilteredDeckError, AnkiError::FilteredDeckError(_) => Kind::FilteredDeckError,
AnkiError::SearchError(_) => Kind::SearchError, AnkiError::SearchError(_) => Kind::SearchError,
AnkiError::TemplateSaveError(_) => Kind::TemplateParse, AnkiError::CardTypeError(_) => Kind::CardTypeError,
AnkiError::ParseNumError => Kind::InvalidInput, AnkiError::ParseNumError => Kind::InvalidInput,
AnkiError::InvalidRegex(_) => Kind::InvalidInput, AnkiError::InvalidRegex(_) => Kind::InvalidInput,
AnkiError::UndoEmpty => Kind::UndoEmpty, AnkiError::UndoEmpty => Kind::UndoEmpty,
@ -43,6 +44,7 @@ impl AnkiError {
pb::BackendError { pb::BackendError {
kind: kind as i32, kind: kind as i32,
localized, localized,
help_page,
} }
} }
} }

View file

@ -14,7 +14,7 @@ pub use network::{NetworkError, NetworkErrorKind, SyncError, SyncErrorKind};
pub use search::{ParseError, SearchErrorKind}; pub use search::{ParseError, SearchErrorKind};
use tempfile::PathPersistError; use tempfile::PathPersistError;
use crate::i18n::I18n; use crate::{i18n::I18n, links::HelpPage};
pub type Result<T, E = AnkiError> = std::result::Result<T, E>; pub type Result<T, E = AnkiError> = std::result::Result<T, E>;
@ -22,7 +22,7 @@ pub type Result<T, E = AnkiError> = std::result::Result<T, E>;
pub enum AnkiError { pub enum AnkiError {
InvalidInput(String), InvalidInput(String),
TemplateError(String), TemplateError(String),
TemplateSaveError(TemplateSaveError), CardTypeError(CardTypeError),
IoError(String), IoError(String),
FileIoError(FileIoError), FileIoError(FileIoError),
DbError(DbError), DbError(DbError),
@ -71,20 +71,17 @@ impl AnkiError {
// already localized // already localized
info.into() info.into()
} }
AnkiError::TemplateSaveError(err) => { AnkiError::CardTypeError(err) => {
let header = let header =
tr.card_templates_invalid_template_number(err.ordinal + 1, &err.notetype); tr.card_templates_invalid_template_number(err.ordinal + 1, &err.notetype);
let details = match err.details { let details = match err.details {
TemplateSaveErrorDetails::TemplateError CardTypeErrorDetails::TemplateError | CardTypeErrorDetails::NoSuchField => {
| TemplateSaveErrorDetails::NoSuchField => tr.card_templates_see_preview(), 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::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!("{}<br>{}", header, details) format!("{}<br>{}", header, details)
} }
@ -120,6 +117,21 @@ impl AnkiError {
} }
} }
} }
pub fn help_page(&self) -> Option<HelpPage> {
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)] #[derive(Debug, PartialEq)]
@ -174,14 +186,14 @@ impl From<regex::Error> for AnkiError {
} }
#[derive(Debug, PartialEq)] #[derive(Debug, PartialEq)]
pub struct TemplateSaveError { pub struct CardTypeError {
pub notetype: String, pub notetype: String,
pub ordinal: usize, pub ordinal: usize,
pub details: TemplateSaveErrorDetails, pub details: CardTypeErrorDetails,
} }
#[derive(Debug, PartialEq)] #[derive(Debug, PartialEq)]
pub enum TemplateSaveErrorDetails { pub enum CardTypeErrorDetails {
TemplateError, TemplateError,
Duplicate(usize), Duplicate(usize),
NoFrontField, NoFrontField,

View file

@ -30,6 +30,17 @@ impl HelpPage {
HelpPage::DeckOptions => "deck-options.html", HelpPage::DeckOptions => "deck-options.html",
HelpPage::EditingFeatures => "editing.html#editing-features", HelpPage::EditingFeatures => "editing.html#editing-features",
HelpPage::FullScreenIssue => "platform/windows/display-issues.html#full-screen", 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"
}
} }
} }
} }

View file

@ -42,7 +42,7 @@ pub use crate::backend_proto::{
}; };
use crate::{ use crate::{
define_newtype, define_newtype,
error::{TemplateSaveError, TemplateSaveErrorDetails}, error::{CardTypeError, CardTypeErrorDetails},
prelude::*, prelude::*,
search::{Node, SearchNode}, search::{Node, SearchNode},
storage::comma_separated_ids, storage::comma_separated_ids,
@ -341,10 +341,10 @@ impl Notetype {
for (index, card) in self.templates.iter().enumerate() { for (index, card) in self.templates.iter().enumerate() {
if let Some(old_index) = map.insert(&card.config.q_format, index) { if let Some(old_index) = map.insert(&card.config.q_format, index) {
if !CARD_TAG.is_match(&card.config.q_format) { if !CARD_TAG.is_match(&card.config.q_format) {
return Err(AnkiError::TemplateSaveError(TemplateSaveError { return Err(AnkiError::CardTypeError(CardTypeError {
notetype: self.name.clone(), notetype: self.name.clone(),
ordinal: index, 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 { if let (Some(q), Some(a)) = sides {
let q_fields = q.fields(); let q_fields = q.fields();
if q_fields.is_empty() { if q_fields.is_empty() {
Some((index, TemplateSaveErrorDetails::NoFrontField)) Some((index, CardTypeErrorDetails::NoFrontField))
} else if self.unknown_field_name(q_fields.union(&a.fields())) { } else if self.unknown_field_name(q_fields.union(&a.fields())) {
Some((index, TemplateSaveErrorDetails::NoSuchField)) Some((index, CardTypeErrorDetails::NoSuchField))
} else { } else {
None None
} }
} else { } else {
Some((index, TemplateSaveErrorDetails::TemplateError)) Some((index, CardTypeErrorDetails::TemplateError))
} }
}) })
{ {
Err(AnkiError::TemplateSaveError(TemplateSaveError { Err(AnkiError::CardTypeError(CardTypeError {
notetype: self.name.clone(), notetype: self.name.clone(),
ordinal: invalid_index, ordinal: invalid_index,
details, details,
@ -405,10 +405,10 @@ impl Notetype {
parsed_templates: &[(Option<ParsedTemplate>, Option<ParsedTemplate>)], parsed_templates: &[(Option<ParsedTemplate>, Option<ParsedTemplate>)],
) -> Result<()> { ) -> Result<()> {
if self.is_cloze() && missing_cloze_filter(parsed_templates) { if self.is_cloze() && missing_cloze_filter(parsed_templates) {
return Err(AnkiError::TemplateSaveError(TemplateSaveError { return Err(AnkiError::CardTypeError(CardTypeError {
notetype: self.name.clone(), notetype: self.name.clone(),
ordinal: 0, ordinal: 0,
details: TemplateSaveErrorDetails::MissingCloze, details: CardTypeErrorDetails::MissingCloze,
})); }));
} }
Ok(()) Ok(())