From f86c2dc567b321f9e5e42d0575ad3f834da7ec6e Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Tue, 21 Apr 2020 16:50:34 +1000 Subject: [PATCH] normal note types now generate a dummy card if required In the cloze deletion case, we already created a dummy card 0 when no cloze deletions were found. This change makes normal note types behave the same way - if no cards would be generated, a dummy card 0 is added to allow the note to be added. This also applies when modifying note types - it is now possible to delete card templates even if some notes only use that template, as a dummy card 0 will be generated for notes that end up with no cards left. --- rslib/src/backend/mod.rs | 1 - rslib/src/err.rs | 3 -- rslib/src/notes.rs | 52 ++++++++++++++--------------- rslib/src/notetype/cardgen.rs | 53 +++++++++++++++++++++--------- rslib/src/notetype/mod.rs | 9 ++++- rslib/src/notetype/schemachange.rs | 1 - rslib/src/notetype/stock.rs | 10 +++--- 7 files changed, 75 insertions(+), 54 deletions(-) diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index bb8d5bf03..0a7f33b8e 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -78,7 +78,6 @@ fn anki_error_to_proto_error(err: AnkiError, i18n: &I18n) -> pb::BackendError { AnkiError::SchemaChange => V::InvalidInput(pb::Empty {}), AnkiError::JSONError { info } => V::JsonError(info), AnkiError::ProtoError { info } => V::ProtoError(info), - AnkiError::NoCardsGenerated => todo!(), }; pb::BackendError { diff --git a/rslib/src/err.rs b/rslib/src/err.rs index cee41d4c7..018fde067 100644 --- a/rslib/src/err.rs +++ b/rslib/src/err.rs @@ -48,9 +48,6 @@ pub enum AnkiError { #[fail(display = "Operation modifies schema, but schema not marked modified.")] SchemaChange, - - #[fail(display = "No cards generated.")] - NoCardsGenerated, } // error helpers diff --git a/rslib/src/notes.rs b/rslib/src/notes.rs index c4e3226c7..2922517ac 100644 --- a/rslib/src/notes.rs +++ b/rslib/src/notes.rs @@ -142,13 +142,8 @@ impl Collection { pub(crate) fn add_note_inner(&mut self, ctx: &CardGenContext, note: &mut Note) -> Result<()> { note.prepare_for_update(&ctx.notetype, ctx.usn)?; - let cards = ctx.new_cards_required(note, Default::default()); - if cards.is_empty() { - return Err(AnkiError::NoCardsGenerated); - } - // add note first, as we need the allocated ID for the cards self.storage.add_note(note)?; - self.add_generated_cards(ctx, note.id, &cards) + self.generate_cards_for_new_note(ctx, note) } pub fn update_note(&mut self, note: &mut Note) -> Result<()> { @@ -177,12 +172,7 @@ impl Collection { #[cfg(test)] mod test { use super::{anki_base91, field_checksum}; - use crate::{ - collection::open_test_collection, - decks::DeckID, - err::{AnkiError, Result}, - search::SortMode, - }; + use crate::{collection::open_test_collection, decks::DeckID, err::Result}; #[test] fn test_base91() { @@ -200,35 +190,43 @@ mod test { } #[test] - fn adding() -> Result<()> { + fn adding_cards() -> Result<()> { let mut col = open_test_collection(); - let nt = col.get_notetype_by_name("basic")?.unwrap(); + let nt = col + .get_notetype_by_name("basic (and reversed card)")? + .unwrap(); let mut note = nt.new_note(); - assert_eq!(col.add_note(&mut note), Err(AnkiError::NoCardsGenerated)); - - note.fields[1] = "foo".into(); - assert_eq!(col.add_note(&mut note), Err(AnkiError::NoCardsGenerated)); - - note.fields[0] = "bar".into(); + // if no cards are generated, 1 card is added col.add_note(&mut note).unwrap(); + let existing = col.storage.existing_cards_for_note(note.id)?; + assert_eq!(existing.len(), 1); + assert_eq!(existing[0].ord, 0); - assert_eq!( - col.search_cards(&format!("nid:{}", note.id), SortMode::NoOrder) - .unwrap() - .len(), - 1 - ); + // nothing changes if the first field is filled + note.fields[0] = "test".into(); + col.update_note(&mut note).unwrap(); + let existing = col.storage.existing_cards_for_note(note.id)?; + assert_eq!(existing.len(), 1); + assert_eq!(existing[0].ord, 0); + // second field causes another card to be generated + note.fields[1] = "test".into(); + col.update_note(&mut note).unwrap(); + let existing = col.storage.existing_cards_for_note(note.id)?; + assert_eq!(existing.len(), 2); + assert_eq!(existing[1].ord, 1); + + // cloze cards also generate card 0 if no clozes are found let nt = col.get_notetype_by_name("cloze")?.unwrap(); let mut note = nt.new_note(); - // cloze cards without any cloze deletions are allowed col.add_note(&mut note).unwrap(); let existing = col.storage.existing_cards_for_note(note.id)?; assert_eq!(existing.len(), 1); assert_eq!(existing[0].ord, 0); assert_eq!(existing[0].original_deck_id, DeckID(1)); + // and generate cards for any cloze deletions note.fields[0] = "{{c1::foo}} {{c2::bar}} {{c3::baz}} {{c0::quux}} {{c501::over}}".into(); col.update_note(&mut note)?; let existing = col.storage.existing_cards_for_note(note.id)?; diff --git a/rslib/src/notetype/cardgen.rs b/rslib/src/notetype/cardgen.rs index f492c1def..dc51cf0dc 100644 --- a/rslib/src/notetype/cardgen.rs +++ b/rslib/src/notetype/cardgen.rs @@ -86,9 +86,20 @@ impl CardGenContext<'_> { existing: &[AlreadyGeneratedCardInfo], ) -> Vec { let extracted = extract_data_from_existing_cards(existing); - match self.notetype.config.kind() { + let cards = match self.notetype.config.kind() { NoteTypeKind::Normal => self.new_cards_required_normal(note, &extracted), NoteTypeKind::Cloze => self.new_cards_required_cloze(note, &extracted), + }; + if extracted.existing_ords.is_empty() && cards.is_empty() { + // if there are no existing cards and no cards will be generated, + // we add card 0 to ensure the note always has at least one card + vec![CardToGenerate { + ord: 0, + did: extracted.deck_id, + due: extracted.due, + }] + } else { + cards } } @@ -128,8 +139,7 @@ impl CardGenContext<'_> { for field in note.fields() { add_cloze_numbers_in_string(field, &mut set); } - let cards: Vec<_> = set - .into_iter() + set.into_iter() .filter_map(|cloze_ord| { let card_ord = cloze_ord.saturating_sub(1).min(499); if extracted.existing_ords.contains(&(card_ord as u32)) { @@ -142,17 +152,7 @@ impl CardGenContext<'_> { }) } }) - .collect(); - if cards.is_empty() && extracted.existing_ords.is_empty() { - // if no cloze deletions are found, we add a card with ord 0 - vec![CardToGenerate { - ord: 0, - did: extracted.deck_id, - due: extracted.due, - }] - } else { - cards - } + .collect() } } @@ -200,12 +200,33 @@ pub(crate) fn extract_data_from_existing_cards( } impl Collection { + pub(crate) fn generate_cards_for_new_note( + &mut self, + ctx: &CardGenContext, + note: &Note, + ) -> Result<()> { + self.generate_cards_for_note(ctx, note, false) + } + pub(crate) fn generate_cards_for_existing_note( &mut self, ctx: &CardGenContext, note: &Note, ) -> Result<()> { - let existing = self.storage.existing_cards_for_note(note.id)?; + self.generate_cards_for_note(ctx, note, true) + } + + fn generate_cards_for_note( + &mut self, + ctx: &CardGenContext, + note: &Note, + check_existing: bool, + ) -> Result<()> { + let existing = if check_existing { + self.storage.existing_cards_for_note(note.id)? + } else { + vec![] + }; let cards = ctx.new_cards_required(note, &existing); if cards.is_empty() { return Ok(()); @@ -225,7 +246,7 @@ impl Collection { continue; } let note = self.storage.get_note(nid)?.unwrap(); - self.generate_cards_for_existing_note(ctx, ¬e)?; + self.generate_cards_for_note(ctx, ¬e, true)?; } Ok(()) diff --git a/rslib/src/notetype/mod.rs b/rslib/src/notetype/mod.rs index 14ba42af2..4cc9a2638 100644 --- a/rslib/src/notetype/mod.rs +++ b/rslib/src/notetype/mod.rs @@ -166,12 +166,19 @@ impl NoteType { self.templates.push(CardTemplate::new(name, qfmt, afmt)); } - pub(crate) fn prepare_for_adding(&mut self) { + pub(crate) fn prepare_for_adding(&mut self) -> Result<()> { // defaults to 0 self.config.target_deck_id = 1; + if self.fields.is_empty() { + return Err(AnkiError::invalid_input("1 field required")); + } + if self.templates.is_empty() { + return Err(AnkiError::invalid_input("1 template required")); + } self.normalize_names(); self.ensure_names_unique(); self.update_requirements(); + Ok(()) } pub fn new_note(&self) -> Note { diff --git a/rslib/src/notetype/schemachange.rs b/rslib/src/notetype/schemachange.rs index 5a5241a45..ca385d794 100644 --- a/rslib/src/notetype/schemachange.rs +++ b/rslib/src/notetype/schemachange.rs @@ -240,5 +240,4 @@ mod test { } } - // fixme: make sure we don't orphan notes diff --git a/rslib/src/notetype/stock.rs b/rslib/src/notetype/stock.rs index a610749ba..6c2fac169 100644 --- a/rslib/src/notetype/stock.rs +++ b/rslib/src/notetype/stock.rs @@ -58,7 +58,7 @@ pub(crate) fn basic(i18n: &I18n) -> NoteType { fieldref(back), ), ); - nt.prepare_for_adding(); + nt.prepare_for_adding().unwrap(); nt } @@ -74,7 +74,7 @@ pub(crate) fn basic_typing(i18n: &I18n) -> NoteType { fieldref(front), back ); - nt.prepare_for_adding(); + nt.prepare_for_adding().unwrap(); nt } @@ -92,7 +92,7 @@ pub(crate) fn basic_forward_reverse(i18n: &I18n) -> NoteType { fieldref(front), ), ); - nt.prepare_for_adding(); + nt.prepare_for_adding().unwrap(); nt } @@ -103,7 +103,7 @@ pub(crate) fn basic_optional_reverse(i18n: &I18n) -> NoteType { nt.add_field(addrev.as_ref()); let tmpl = &mut nt.templates[1].config; tmpl.q_format = format!("{{{{#{}}}}}{}{{{{/{}}}}}", addrev, tmpl.q_format, addrev); - nt.prepare_for_adding(); + nt.prepare_for_adding().unwrap(); nt } @@ -124,6 +124,6 @@ pub(crate) fn cloze(i18n: &I18n) -> NoteType { color: lightblue; } "; - nt.prepare_for_adding(); + nt.prepare_for_adding().unwrap(); nt }