Merge pull request #1230 from RumovZ/fields-check

Check for misplaced or missing clozes when adding and in the editor

ftl/core/adding.ftl

@@ -6,4 +6,6 @@ adding-history = History
 adding-note-deleted = (Note deleted)
 adding-shortcut = Shortcut: { $val }
 adding-the-first-field-is-empty = The first field is empty.
-adding-you-have-a-cloze-deletion-note = You have a cloze deletion note type but have not made any cloze deletions. Proceed?
+adding-you-have-a-cloze-deletion-note = You have a cloze notetype but have not made any cloze deletions. Proceed?
+adding-cloze-outside-cloze-notetype = Cloze deletion can only be used on cloze notetypes.
+adding-cloze-outside-cloze-field = Cloze deletion can only be used in fields which use the 'cloze:' filter. This is typically the first field.

pylib/.pylintrc

@@ -11,7 +11,7 @@ ignored-classes=
   QueuedCards,
   UnburyDeckIn,
   BuryOrSuspendCardsIn,
-  NoteIsDuplicateOrEmptyOut,
+  NoteFieldsCheckOut,
   BackendError,
   SetDeckCollapsedIn,
 

pylib/anki/notes.py

@@ -14,7 +14,8 @@ from anki.consts import MODEL_STD
 from anki.models import NotetypeDict, NotetypeId, TemplateDict
 from anki.utils import joinFields
 
-DuplicateOrEmptyResult = _pb.NoteIsDuplicateOrEmptyOut.State
+DuplicateOrEmptyResult = _pb.NoteFieldsCheckOut.State
+NoteFieldsCheckResult = _pb.NoteFieldsCheckOut.State
 
 # types
 NoteId = NewType("NoteId", int)
@@ -190,12 +191,10 @@ class Note:
     addTag = add_tag
     delTag = remove_tag
 
-    # Unique/duplicate check
+    # Unique/duplicate/cloze check
     ##################################################
 
-    def duplicate_or_empty(self) -> DuplicateOrEmptyResult.V:
-        return self.col._backend.note_is_duplicate_or_empty(
-            self._to_backend_note()
-        ).state
+    def fields_check(self) -> NoteFieldsCheckResult.V:
+        return self.col._backend.note_fields_check(self._to_backend_note()).state
 
-    dupeOrEmpty = duplicate_or_empty
+    dupeOrEmpty = duplicate_or_empty = fields_check

pylib/tests/test_collection.py

@@ -74,15 +74,15 @@ def test_noteAddDelete():
     c0 = note.cards()[0]
     assert "three" in c0.q()
     # it should not be a duplicate
-    assert not note.duplicate_or_empty()
+    assert not note.fields_check()
     # now let's make a duplicate
     note2 = col.newNote()
     note2["Front"] = "one"
     note2["Back"] = ""
-    assert note2.duplicate_or_empty()
+    assert note2.fields_check()
     # empty first field should not be permitted either
     note2["Front"] = " "
-    assert note2.duplicate_or_empty()
+    assert note2.fields_check()
 
 
 def test_fieldChecksum():

qt/aqt/addcards.py

@@ -6,10 +6,9 @@ from typing import Callable, List, Optional
 import aqt.editor
 import aqt.forms
 from anki.collection import OpChanges, SearchNode
-from anki.consts import MODEL_CLOZE
 from anki.decks import DeckId
 from anki.models import NotetypeId
-from anki.notes import DuplicateOrEmptyResult, Note, NoteId
+from anki.notes import Note, NoteFieldsCheckResult, NoteId
 from anki.utils import htmlToTextLine, isMac
 from aqt import AnkiQt, gui_hooks
 from aqt.deckchooser import DeckChooser
@@ -231,9 +230,16 @@ class AddCards(QDialog):
         ).run_in_background()
 
     def _note_can_be_added(self, note: Note) -> bool:
-        result = note.duplicate_or_empty()
-        if result == DuplicateOrEmptyResult.EMPTY:
+        result = note.fields_check()
+        if result == NoteFieldsCheckResult.EMPTY:
             problem = tr.adding_the_first_field_is_empty()
+        elif result == NoteFieldsCheckResult.MISSING_CLOZE:
+            if not askUser(tr.adding_you_have_a_cloze_deletion_note()):
+                return False
+        elif result == NoteFieldsCheckResult.NOTETYPE_NOT_CLOZE:
+            problem = tr.adding_cloze_outside_cloze_notetype()
+        elif result == NoteFieldsCheckResult.FIELD_NOT_CLOZE:
+            problem = tr.adding_cloze_outside_cloze_field()
         else:
             # duplicate entries are allowed these days
             problem = None
@@ -244,12 +250,6 @@ class AddCards(QDialog):
             showWarning(problem, help=HelpPage.ADDING_CARD_AND_NOTE)
             return False
 
-        # missing cloze deletion?
-        if note.model()["type"] == MODEL_CLOZE:
-            if not note.cloze_numbers_in_fields():
-                if not askUser(tr.adding_you_have_a_cloze_deletion_note()):
-                    return False
-
         return True
 
     def keyPressEvent(self, evt: QKeyEvent) -> None:

qt/aqt/editor.py

@@ -27,7 +27,7 @@ from anki.collection import Config, SearchNode
 from anki.consts import MODEL_CLOZE
 from anki.hooks import runFilter
 from anki.httpclient import HttpClient
-from anki.notes import DuplicateOrEmptyResult, Note
+from anki.notes import Note, NoteFieldsCheckResult
 from anki.utils import checksum, isLin, isWin, namedtmp
 from aqt import AnkiQt, colors, gui_hooks
 from aqt.operations import QueryOp
@@ -79,8 +79,9 @@ audio = (
 _html = """
 <div id="fields"></div>
 <div id="dupes" class="is-inactive">
-    <a href="#" onclick="pycmd('dupes');return false;">%s</a>
+    <a href="#" onclick="pycmd('dupes');return false;">{}</a>
 </div>
+<div id="cloze-hint"></div>
 """
 
 
@@ -129,7 +130,7 @@ class Editor:
 
         # then load page
         self.web.stdHtml(
-            _html % tr.editing_show_duplicates(),
+            _html.format(tr.editing_show_duplicates()),
             css=[
                 "css/editor.css",
             ],
@@ -437,7 +438,7 @@ $editorToolbar.then(({{ toolbar }}) => toolbar.appendGroup({{
         self.widget.show()
         self.updateTags()
 
-        dupe_status = self.note.duplicate_or_empty()
+        note_fields_status = self.note.fields_check()
 
         def oncallback(arg: Any) -> None:
             if not self.note:
@@ -445,7 +446,7 @@ $editorToolbar.then(({{ toolbar }}) => toolbar.appendGroup({{
             self.setupForegroundButton()
             # we currently do this synchronously to ensure we load before the
             # sidebar on browser startup
-            self._update_duplicate_display(dupe_status)
+            self._update_duplicate_display(note_fields_status)
             if focusTo is not None:
                 self.web.setFocus()
             gui_hooks.editor_did_load_note(self)
@@ -494,25 +495,31 @@ $editorToolbar.then(({{ toolbar }}) => toolbar.appendGroup({{
         if not note:
             return
 
-        def on_done(result: DuplicateOrEmptyResult.V) -> None:
+        def on_done(result: NoteFieldsCheckResult.V) -> None:
             if self.note != note:
                 return
             self._update_duplicate_display(result)
 
         QueryOp(
             parent=self.parentWindow,
-            op=lambda _: note.duplicate_or_empty(),
+            op=lambda _: note.fields_check(),
             success=on_done,
         ).run_in_background()
 
     checkValid = _check_and_update_duplicate_display_async
 
-    def _update_duplicate_display(self, result: DuplicateOrEmptyResult.V) -> None:
+    def _update_duplicate_display(self, result: NoteFieldsCheckResult.V) -> None:
         cols = [""] * len(self.note.fields)
-        if result == DuplicateOrEmptyResult.DUPLICATE:
+        cloze_hint = ""
+        if result == NoteFieldsCheckResult.DUPLICATE:
             cols[0] = "dupe"
+        elif result == NoteFieldsCheckResult.NOTETYPE_NOT_CLOZE:
+            cloze_hint = tr.adding_cloze_outside_cloze_notetype()
+        elif result == NoteFieldsCheckResult.FIELD_NOT_CLOZE:
+            cloze_hint = tr.adding_cloze_outside_cloze_field()
 
         self.web.eval(f"setBackgrounds({json.dumps(cols)});")
+        self.web.eval(f"setClozeHint({json.dumps(cloze_hint)});")
 
     def showDupes(self) -> None:
         aqt.dialogs.open(

rslib/backend.proto

@@ -174,7 +174,7 @@ service NotesService {
   rpc ClozeNumbersInNote(Note) returns (ClozeNumbersInNoteOut);
   rpc AfterNoteUpdates(AfterNoteUpdatesIn) returns (OpChangesWithCount);
   rpc FieldNamesForNotes(FieldNamesForNotesIn) returns (FieldNamesForNotesOut);
-  rpc NoteIsDuplicateOrEmpty(Note) returns (NoteIsDuplicateOrEmptyOut);
+  rpc NoteFieldsCheck(Note) returns (NoteFieldsCheckOut);
   rpc CardsOfNote(NoteId) returns (CardIds);
 }
 
@@ -1231,11 +1231,14 @@ message ReparentDecksIn {
   int64 new_parent = 2;
 }
 
-message NoteIsDuplicateOrEmptyOut {
+message NoteFieldsCheckOut {
   enum State {
     NORMAL = 0;
     EMPTY = 1;
     DUPLICATE = 2;
+    MISSING_CLOZE = 3;
+    NOTETYPE_NOT_CLOZE = 4;
+    FIELD_NOT_CLOZE = 5;
   }
   State state = 1;
 }

rslib/src/backend/notes.rs

@@ -120,11 +120,11 @@ impl NotesService for Backend {
         })
     }
 
-    fn note_is_duplicate_or_empty(&self, input: pb::Note) -> Result<pb::NoteIsDuplicateOrEmptyOut> {
+    fn note_fields_check(&self, input: pb::Note) -> Result<pb::NoteFieldsCheckOut> {
         let note: Note = input.into();
         self.with_col(|col| {
-            col.note_is_duplicate_or_empty(&note)
-                .map(|r| pb::NoteIsDuplicateOrEmptyOut { state: r as i32 })
+            col.note_fields_check(&note)
+                .map(|r| pb::NoteFieldsCheckOut { state: r as i32 })
         })
     }
 

rslib/src/cloze.rs

@@ -139,6 +139,10 @@ pub fn expand_clozes_to_reveal_latex(text: &str) -> String {
     buf
 }
 
+pub(crate) fn contains_cloze(text: &str) -> bool {
+    CLOZE.is_match(text)
+}
+
 pub fn cloze_numbers_in_string(html: &str) -> HashSet<u16> {
     let mut set = HashSet::with_capacity(4);
     add_cloze_numbers_in_string(html, &mut set);

rslib/src/notes/mod.rs

@@ -14,7 +14,8 @@ use num_integer::Integer;
 
 use crate::{
     backend_proto as pb,
-    backend_proto::note_is_duplicate_or_empty_out::State as DuplicateState,
+    backend_proto::note_fields_check_out::State as NoteFieldsState,
+    cloze::contains_cloze,
     decks::DeckId,
     define_newtype,
     error::{AnkiError, Result},
@@ -529,35 +530,76 @@ impl Collection {
         Ok(changed_notes)
     }
 
-    pub(crate) fn note_is_duplicate_or_empty(&self, note: &Note) -> Result<DuplicateState> {
-        if let Some(field1) = note.fields.get(0) {
+    /// Check if the note's first field is empty or a duplicate. Then for cloze
+    /// notetypes, check if there is a cloze in a non-cloze field or if there's
+    /// no cloze at all. For other notetypes, just check if there's a cloze.
+    pub(crate) fn note_fields_check(&mut self, note: &Note) -> Result<NoteFieldsState> {
+        Ok(if let Some(text) = note.fields.get(0) {
             let field1 = if self.get_config_bool(BoolKey::NormalizeNoteText) {
-                normalize_to_nfc(field1)
+                normalize_to_nfc(text)
             } else {
-                field1.into()
+                text.into()
             };
             let stripped = strip_html_preserving_media_filenames(&field1);
             if stripped.trim().is_empty() {
-                Ok(DuplicateState::Empty)
+                NoteFieldsState::Empty
             } else {
-                let csum = field_checksum(&stripped);
-                let have_dupe = self
-                    .storage
-                    .note_fields_by_checksum(note.notetype_id, csum)?
-                    .into_iter()
-                    .any(|(nid, field)| {
-                        nid != note.id && strip_html_preserving_media_filenames(&field) == stripped
-                    });
-
-                if have_dupe {
-                    Ok(DuplicateState::Duplicate)
+                let cloze_state = self.field_cloze_check(note)?;
+                if cloze_state != NoteFieldsState::Normal {
+                    cloze_state
+                } else if self.is_duplicate(&stripped, note)? {
+                    NoteFieldsState::Duplicate
                 } else {
-                    Ok(DuplicateState::Normal)
+                    NoteFieldsState::Normal
                 }
             }
         } else {
-            Ok(DuplicateState::Empty)
-        }
+            NoteFieldsState::Empty
+        })
+    }
+
+    fn is_duplicate(&self, first_field: &str, note: &Note) -> Result<bool> {
+        let csum = field_checksum(&first_field);
+        Ok(self
+            .storage
+            .note_fields_by_checksum(note.notetype_id, csum)?
+            .into_iter()
+            .any(|(nid, field)| {
+                nid != note.id && strip_html_preserving_media_filenames(&field) == first_field
+            }))
+    }
+
+    fn field_cloze_check(&mut self, note: &Note) -> Result<NoteFieldsState> {
+        let notetype = self
+            .get_notetype(note.notetype_id)?
+            .ok_or(AnkiError::NotFound)?;
+        let cloze_fields = notetype.cloze_fields();
+        let mut has_cloze = false;
+        let extraneous_cloze = note.fields.iter().enumerate().find_map(|(i, field)| {
+            if notetype.is_cloze() {
+                if contains_cloze(field) {
+                    if cloze_fields.contains(&i) {
+                        has_cloze = true;
+                        None
+                    } else {
+                        Some(NoteFieldsState::FieldNotCloze)
+                    }
+                } else {
+                    None
+                }
+            } else if contains_cloze(field) {
+                Some(NoteFieldsState::NotetypeNotCloze)
+            } else {
+                None
+            }
+        });
+        Ok(if let Some(state) = extraneous_cloze {
+            state
+        } else if notetype.is_cloze() && !has_cloze {
+            NoteFieldsState::MissingCloze
+        } else {
+            NoteFieldsState::Normal
+        })
     }
 }
 

rslib/src/notetype/mod.rs

@@ -548,6 +548,22 @@ impl Notetype {
     pub(crate) fn is_cloze(&self) -> bool {
         matches!(self.config.kind(), NotetypeKind::Cloze)
     }
+
+    /// Return all clozable fields. A field is clozable when it belongs to a cloze
+    /// notetype and a 'cloze' filter is applied to it in the template.
+    pub(crate) fn cloze_fields(&self) -> HashSet<usize> {
+        if !self.is_cloze() {
+            HashSet::new()
+        } else if let Some((Some(front), _)) = self.parsed_templates().get(0) {
+            front
+                .cloze_fields()
+                .iter()
+                .filter_map(|name| self.get_field_ord(name))
+                .collect()
+        } else {
+            HashSet::new()
+        }
+    }
 }
 
 /// True if the slice is empty or either template of the first tuple doesn't have a cloze field.

ts/editor/fields.scss

@@ -28,7 +28,8 @@
     padding: 0;
 }
 
-#dupes {
+#dupes,
+#cloze-hint {
     position: sticky;
     bottom: 0;
 

ts/editor/index.ts

@@ -144,6 +144,10 @@ export function setBackgrounds(cols: ("dupe" | "")[]): void {
         .classList.toggle("is-inactive", !cols.includes("dupe"));
 }
 
+export function setClozeHint(cloze_hint: string): void {
+    document.getElementById("cloze-hint")!.innerHTML = cloze_hint;
+}
+
 export function setFonts(fonts: [string, number, boolean][]): void {
     forEditorField(
         fonts,