From 9686cd99ec13ed9be711abd19b1520d5ec9a7b8c Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sat, 16 Jan 2021 15:37:40 +1000 Subject: [PATCH] search error tweaks - use markdown instead of HTML, to make editing and translating easier - use a shared prefix - a few very minor wording tweaks - we don't need to translate undocumented command errors - share a string for positive number of days - share a string for invalid property and state arguments, and avoid listing them out Related discussion: https://github.com/ankitects/anki/pull/922 --- ftl/core/search.ftl | 52 ++++++------- qt/aqt/browser.py | 25 +++++-- rslib/src/err.rs | 146 ++++++++++++++++++++++--------------- rslib/src/search/parser.rs | 20 ++--- 4 files changed, 140 insertions(+), 103 deletions(-) diff --git a/ftl/core/search.ftl b/ftl/core/search.ftl index eb266b5ad..1869741bc 100644 --- a/ftl/core/search.ftl +++ b/ftl/core/search.ftl @@ -1,34 +1,30 @@ ## Errors shown when invalid search input is encountered. -## Text wrapped in code tags is literal search input and should generally not to be altered. +## Text wrapped in `backticks` is literal search input, and should generally not to be altered. -search-invalid = Invalid search - please check for typing mistakes. -search-misplaced-and = Invalid search:
An and was found but it is not connecting two search terms.
If you want to search for the word itself, wrap it in double quotes: "and". -search-misplaced-or = Invalid search:
An or was found but it is not connecting two search terms.
If you want to search for the word itself, wrap it in double quotes: "or". +search-invalid-search = Invalid search: { $reason } +search-misplaced-and = an `and` was found but it is not connecting two search terms. If you want to search for the word itself, wrap it in double quotes: `"and"`. +search-misplaced-or = an `or` was found but it is not connecting two search terms. If you want to search for the word itself, wrap it in double quotes: `"or"`. # Here, the ellipsis "..." may be localised. -search-empty-group = Invalid search:
A group (...) was found but there was nothing between the brackets to search for.
If you want to search for literal brackets, wrap them in double quotes: "( )". -search-unopened-group = Invalid search:
A closing bracket ) was found, but there was no opening bracket ( preceding it.
If you want to search for the literal ), wrap it in double quotes or prepend a backslash: ")" or \). -search-unclosed-group = Invalid search:
An opening bracket ( was found, but there was no closing bracket ) following it.
If you want to search for the literal (, wrap it in double quotes or prepend a backslash: "(" or \( . -search-empty-quote = Invalid search:
A pair of double quotes "" was found but there was nothing between them to search for.
If you want to search for literal double quotes, prepend backslashes: \"\". -search-unclosed-quote = Invalid search:
An opening double quote " was found but there was no second one to close it.
If you want to search for the literal ", prepend a backslash: \". -search-missing-key = Invalid search:
A colon : was found but there was no key word preceding it.
If you want to search for the literal :, prepend a backslash: \:. -search-unknown-escape = Invalid search:
The escape sequence { $val } is not defined.
If you want to search for the literal backslash \, prepend another one: \\. -search-invalid-id-list = Invalid search:
Note or card id lists must be comma-separated number series. -search-invalid-state = Invalid search:
is: must be followed by one of the predefined card states: new, review, learn, due, buried, buried-manually, buried-sibling or suspended. -search-invalid-flag = Invalid search:
flag: must be followed by a valid flag number: 1 (red), 2 (orange), 3 (green), 4 (blue) or 0 (no flag). -search-invalid-added = Invalid search:
added: must be followed by a positive number of days. -search-invalid-edited = Invalid search:
edited: must be followed by a positive number of days. -search-invalid-rated-days = Invalid search:
rated: must be followed by a positive number of days. -search-invalid-rated-ease = Invalid search:
rated:{ $val }: must be followed by 1 (again), 2 (hard), 3 (good) or 4 (easy). -search-invalid-resched = Invalid search:
resched: must be followed by a positive number of days. -search-invalid-dupe-mid = Invalid search:
dupe: must be followed by a note type id, a comma and then arbitrary text. -search-invalid-dupe-text = Invalid search:
dupe: must be followed by a note type id, a comma and then arbitrary text. -search-invalid-prop-property = Invalid search:
prop: must be followed by one of the predefined card properties: ivl (interval), due, reps (repetitions), lapses, ease or pos (position). -search-invalid-prop-operator = Invalid search:
prop:{ $val } must be followed by one of the comparison operators: =, !=, <, >, <= or >=. -search-invalid-prop-float = Invalid search:
prop:{ $val } must be followed by a decimal number. -search-invalid-prop-integer = Invalid search:
prop:{ $val } must be followed by a whole number. -search-invalid-prop-unsigned = Invalid search:
prop:{ $val } must be followed by a non-negative whole number. -search-invalid-did = Invalid search:
did: must be followed by a valid deck id. -search-invalid-mid = Invalid search:
mid: must be followed by a note type deck id. +search-empty-group = a group `(...)` was found, but there was nothing between the brackets to search for. If you want to search for literal brackets, wrap them in double quotes: `"( )"`. +search-unopened-group = a closing bracket `)` was found, but there was no opening bracket `(` preceding it. If you want to search for the literal `)`, wrap it in double quotes or prepend a backslash: `")"` or `\)`. +search-unclosed-group = an opening bracket `(` was found, but there was no closing bracket `)` following it. If you want to search for the literal `(`, wrap it in double quotes or prepend a backslash: `"("` or `\(` . +search-empty-quote = a pair of double quotes `""` was found but there was nothing between them to search for. If you want to search for literal double quotes, prepend backslashes: `\"\"`. +search-unclosed-quote = an opening double quote `"` was found but there was no second one to close it. If you want to search for the literal `"`, prepend a backslash: `\"`. +search-missing-key = a colon `:` was found but there was no keyword preceding it. If you want to search for the literal `:`, prepend a backslash: `\:`. +search-unknown-escape = the escape sequence `{ $val }` is not defined. If you want to search for the literal backslash `\`, prepend another one: `\\`. +search-invalid-id-list = note or card id lists must be comma-separated numbers. +search-invalid-argument = `{ $term }` was given an invalid argument '`{ $argument }`'. +search-invalid-flag = `flag:` must be followed by a valid flag number: `1` (red), `2` (orange), `3` (green), `4` (blue) or `0` (no flag). +search-invalid-followed-by-positive-days = `{ $term }` must be followed by a positive number of days. +search-invalid-rated-days = `rated:` must be followed by a positive number of days. +search-invalid-rated-ease = `rated:{ $val }:` must be followed by `1` (again), `2` (hard), `3` (good) or `4` (easy). +search-invalid-prop-operator = `prop:{ $val }` must be followed by one of the comparison operators: `=`, `!=`, `<`, `>`, `<=` or `>=`. +search-invalid-prop-float = `prop:{ $val }` must be followed by a decimal number. +search-invalid-prop-integer = `prop:{ $val }` must be followed by a whole number. +search-invalid-prop-unsigned = `prop:{ $val }` must be followed by a non-negative whole number. +search-invalid-did = `did:` must be followed by a valid deck id. +search-invalid-mid = `mid:` must be followed by a note type id. +search-invalid-other = please check for typing mistakes. ## Column labels in browse screen diff --git a/qt/aqt/browser.py b/qt/aqt/browser.py index 6ecacc6d0..cd6ab9dea 100644 --- a/qt/aqt/browser.py +++ b/qt/aqt/browser.py @@ -11,6 +11,8 @@ from enum import Enum from operator import itemgetter from typing import Callable, List, Optional, Sequence, Tuple, Union, cast +from markdown import markdown + import anki import aqt import aqt.forms @@ -88,6 +90,14 @@ class SearchContext: card_ids: Optional[Sequence[int]] = None +def show_invalid_search_error(err: Exception): + "Render search errors in markdown, then display a warning." + text = str(err) + if isinstance(err, InvalidInput): + text = markdown(text) + showWarning(text) + + # Data model ########################################################################## @@ -191,7 +201,7 @@ class DataModel(QAbstractTableModel): def search(self, txt: str) -> None: self.beginReset() self.cards = [] - error_message: Optional[str] = None + exception: Optional[Exception] = None try: ctx = SearchContext(search=txt, browser=self.browser) gui_hooks.browser_will_search(ctx) @@ -201,12 +211,12 @@ class DataModel(QAbstractTableModel): gui_hooks.browser_did_search(ctx) self.cards = ctx.card_ids except Exception as e: - error_message = str(e) + exception = e finally: self.endReset() - if error_message: - showWarning(error_message) + if exception: + show_invalid_search_error(exception) def reset(self): self.beginReset() @@ -1252,7 +1262,7 @@ QTableView {{ gridline-color: {grid} }} searches=[cur, search], ) except InvalidInput as e: - showWarning(str(e)) + show_invalid_search_error(e) else: self.form.searchEdit.lineEdit().setText(search) self.onSearchActivated() @@ -1446,7 +1456,7 @@ QTableView {{ gridline-color: {grid} }} self.form.searchEdit.lineEdit().text() ) except InvalidInput as e: - showWarning(str(e)) + show_invalid_search_error(e) else: name = getOnlyText(tr(TR.BROWSING_PLEASE_GIVE_YOUR_FILTER_A_NAME)) if not name: @@ -2001,8 +2011,7 @@ where id in %s""" try: changed = fut.result() except InvalidInput as e: - # failed regex - showWarning(str(e)) + show_invalid_search_error(e) return showInfo( diff --git a/rslib/src/err.rs b/rslib/src/err.rs index a38052bd9..3f321fbbe 100644 --- a/rslib/src/err.rs +++ b/rslib/src/err.rs @@ -123,62 +123,94 @@ impl AnkiError { DBErrorKind::Locked => "Anki already open, or media currently syncing.".into(), _ => format!("{:?}", self), }, - AnkiError::SearchError(kind) => match kind { - SearchErrorKind::MisplacedAnd => i18n.tr(TR::SearchMisplacedAnd), - SearchErrorKind::MisplacedOr => i18n.tr(TR::SearchMisplacedOr), - SearchErrorKind::EmptyGroup => i18n.tr(TR::SearchEmptyGroup), - SearchErrorKind::UnopenedGroup => i18n.tr(TR::SearchUnopenedGroup), - SearchErrorKind::UnclosedGroup => i18n.tr(TR::SearchUnclosedGroup), - SearchErrorKind::EmptyQuote => i18n.tr(TR::SearchEmptyQuote), - SearchErrorKind::UnclosedQuote => i18n.tr(TR::SearchUnclosedQuote), - SearchErrorKind::MissingKey => i18n.tr(TR::SearchMissingKey), - SearchErrorKind::UnknownEscape(ctx) => i18n - .trn( - TR::SearchUnknownEscape, - tr_strs!["val"=>(htmlescape::encode_minimal(ctx))], - ) - .into(), - SearchErrorKind::InvalidIdList => i18n.tr(TR::SearchInvalidIdList), - SearchErrorKind::InvalidState => i18n.tr(TR::SearchInvalidState), - SearchErrorKind::InvalidFlag => i18n.tr(TR::SearchInvalidFlag), - SearchErrorKind::InvalidAdded => i18n.tr(TR::SearchInvalidAdded), - SearchErrorKind::InvalidEdited => i18n.tr(TR::SearchInvalidEdited), - SearchErrorKind::InvalidRatedDays => i18n.tr(TR::SearchInvalidRatedDays), - SearchErrorKind::InvalidRatedEase(ctx) => i18n - .trn(TR::SearchInvalidRatedEase, tr_strs!["val"=>(ctx)]) - .into(), - SearchErrorKind::InvalidResched => i18n.tr(TR::SearchInvalidResched), - SearchErrorKind::InvalidDupeMid => i18n.tr(TR::SearchInvalidDupeMid), - SearchErrorKind::InvalidDupeText => i18n.tr(TR::SearchInvalidDupeText), - SearchErrorKind::InvalidPropProperty => i18n.tr(TR::SearchInvalidPropProperty), - SearchErrorKind::InvalidPropOperator(ctx) => i18n - .trn(TR::SearchInvalidPropOperator, tr_strs!["val"=>(ctx)]) - .into(), - SearchErrorKind::InvalidPropFloat(ctx) => i18n - .trn( - TR::SearchInvalidPropFloat, - tr_strs!["val"=>(htmlescape::encode_minimal(ctx))], - ) - .into(), - SearchErrorKind::InvalidPropInteger(ctx) => i18n - .trn( - TR::SearchInvalidPropInteger, - tr_strs!["val"=>(htmlescape::encode_minimal(ctx))], - ) - .into(), - SearchErrorKind::InvalidPropUnsigned(ctx) => i18n - .trn( - TR::SearchInvalidPropUnsigned, - tr_strs!["val"=>(htmlescape::encode_minimal(ctx))], - ) - .into(), - SearchErrorKind::InvalidDid => i18n.tr(TR::SearchInvalidDid), - SearchErrorKind::InvalidMid => i18n.tr(TR::SearchInvalidMid), - SearchErrorKind::Regex(text) => text.into(), - SearchErrorKind::Other(Some(info)) => info.into(), - SearchErrorKind::Other(None) => i18n.tr(TR::SearchInvalid), + AnkiError::SearchError(kind) => { + let reason = match kind { + SearchErrorKind::MisplacedAnd => i18n.tr(TR::SearchMisplacedAnd), + SearchErrorKind::MisplacedOr => i18n.tr(TR::SearchMisplacedOr), + SearchErrorKind::EmptyGroup => i18n.tr(TR::SearchEmptyGroup), + SearchErrorKind::UnopenedGroup => i18n.tr(TR::SearchUnopenedGroup), + SearchErrorKind::UnclosedGroup => i18n.tr(TR::SearchUnclosedGroup), + SearchErrorKind::EmptyQuote => i18n.tr(TR::SearchEmptyQuote), + SearchErrorKind::UnclosedQuote => i18n.tr(TR::SearchUnclosedQuote), + SearchErrorKind::MissingKey => i18n.tr(TR::SearchMissingKey), + SearchErrorKind::UnknownEscape(ctx) => i18n + .trn( + TR::SearchUnknownEscape, + tr_strs!["val"=>(htmlescape::encode_minimal(ctx))], + ) + .into(), + SearchErrorKind::InvalidIdList => i18n.tr(TR::SearchInvalidIdList), + SearchErrorKind::InvalidState(state) => i18n + .trn( + TR::SearchInvalidArgument, + tr_strs!("term" => "is:", "argument" => state), + ) + .into(), + SearchErrorKind::InvalidFlag => i18n.tr(TR::SearchInvalidFlag), + SearchErrorKind::InvalidAdded => i18n + .trn( + TR::SearchInvalidFollowedByPositiveDays, + tr_strs!("term" => "added:"), + ) + .into(), + SearchErrorKind::InvalidEdited => i18n + .trn( + TR::SearchInvalidFollowedByPositiveDays, + tr_strs!("term" => "edited:"), + ) + .into(), + SearchErrorKind::InvalidRatedDays => i18n.tr(TR::SearchInvalidRatedDays), + SearchErrorKind::InvalidRatedEase(ctx) => i18n + .trn(TR::SearchInvalidRatedEase, tr_strs!["val"=>(ctx)]) + .into(), + SearchErrorKind::InvalidResched => i18n + .trn( + TR::SearchInvalidFollowedByPositiveDays, + tr_strs!("term" => "resched:"), + ) + .into(), + SearchErrorKind::InvalidDupeMid | SearchErrorKind::InvalidDupeText => { + // this is an undocumented search keyword, so no translation + "`dupe:` arguments were invalid".into() + } + SearchErrorKind::InvalidPropProperty(prop) => i18n + .trn( + TR::SearchInvalidArgument, + tr_strs!("term" => "prop:", "argument" => prop), + ) + .into(), + SearchErrorKind::InvalidPropOperator(ctx) => i18n + .trn(TR::SearchInvalidPropOperator, tr_strs!["val"=>(ctx)]) + .into(), + SearchErrorKind::InvalidPropFloat(ctx) => i18n + .trn( + TR::SearchInvalidPropFloat, + tr_strs!["val"=>(htmlescape::encode_minimal(ctx))], + ) + .into(), + SearchErrorKind::InvalidPropInteger(ctx) => i18n + .trn( + TR::SearchInvalidPropInteger, + tr_strs!["val"=>(htmlescape::encode_minimal(ctx))], + ) + .into(), + SearchErrorKind::InvalidPropUnsigned(ctx) => i18n + .trn( + TR::SearchInvalidPropUnsigned, + tr_strs!["val"=>(htmlescape::encode_minimal(ctx))], + ) + .into(), + SearchErrorKind::InvalidDid => i18n.tr(TR::SearchInvalidDid), + SearchErrorKind::InvalidMid => i18n.tr(TR::SearchInvalidMid), + SearchErrorKind::Regex(text) => text.into(), + SearchErrorKind::Other(Some(info)) => info.into(), + SearchErrorKind::Other(None) => i18n.tr(TR::SearchInvalidOther), + }; + i18n.trn( + TR::SearchInvalidSearch, + tr_args!("reason" => reason.into_owned()), + ) } - .into(), _ => format!("{:?}", self), } } @@ -408,7 +440,7 @@ pub enum SearchErrorKind { MissingKey, UnknownEscape(String), InvalidIdList, - InvalidState, + InvalidState(String), InvalidFlag, InvalidAdded, InvalidEdited, @@ -417,7 +449,7 @@ pub enum SearchErrorKind { InvalidDupeMid, InvalidDupeText, InvalidResched, - InvalidPropProperty, + InvalidPropProperty(String), InvalidPropOperator(String), InvalidPropFloat(String), InvalidPropInteger(String), diff --git a/rslib/src/search/parser.rs b/rslib/src/search/parser.rs index 27f7cad34..ff7f1ab84 100644 --- a/rslib/src/search/parser.rs +++ b/rslib/src/search/parser.rs @@ -370,7 +370,7 @@ fn parse_prop(s: &str) -> ParseResult { tag("ease"), tag("pos"), ))(s) - .map_err(|_| parse_failure(s, FailKind::InvalidPropProperty))?; + .map_err(|_| parse_failure(s, FailKind::InvalidPropProperty(s.into())))?; let (num, operator) = alt::<_, _, ParseError, _>(( tag("<="), @@ -482,7 +482,7 @@ fn parse_state(s: &str) -> ParseResult { "buried-manually" => UserBuried, "buried-sibling" => SchedBuried, "suspended" => Suspended, - _ => return Err(parse_failure(s, FailKind::InvalidState)), + _ => return Err(parse_failure(s, FailKind::InvalidState(s.into()))), })) } @@ -845,11 +845,11 @@ mod test { assert_err_kind("cid:,2,3", InvalidIdList); assert_err_kind("cid:1,2,", InvalidIdList); - assert_err_kind("is:foo", InvalidState); - assert_err_kind("is:DUE", InvalidState); - assert_err_kind("is:New", InvalidState); - assert_err_kind("is:", InvalidState); - assert_err_kind(r#""is:learn ""#, InvalidState); + assert_err_kind("is:foo", InvalidState("foo".into())); + assert_err_kind("is:DUE", InvalidState("DUE".into())); + assert_err_kind("is:New", InvalidState("New".into())); + assert_err_kind("is:", InvalidState("".into())); + assert_err_kind(r#""is:learn ""#, InvalidState("learn ".into())); assert_err_kind(r#""flag: ""#, InvalidFlag); assert_err_kind("flag:-0", InvalidFlag); @@ -888,9 +888,9 @@ mod test { assert_err_kind("dupe:123", InvalidDupeText); - assert_err_kind("prop:", InvalidPropProperty); - assert_err_kind("prop:=1", InvalidPropProperty); - assert_err_kind("prop:DUE<5", InvalidPropProperty); + assert_err_kind("prop:", InvalidPropProperty("".into())); + assert_err_kind("prop:=1", InvalidPropProperty("=1".into())); + assert_err_kind("prop:DUE<5", InvalidPropProperty("DUE<5".into())); assert_err_kind("prop:lapses", InvalidPropOperator("lapses".to_string())); assert_err_kind("prop:pos~1", InvalidPropOperator("pos".to_string()));