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.
This commit is contained in:
Damien Elmes 2020-04-21 16:50:34 +10:00
parent 5c3e5c672f
commit f86c2dc567
7 changed files with 75 additions and 54 deletions

View file

@ -78,7 +78,6 @@ fn anki_error_to_proto_error(err: AnkiError, i18n: &I18n) -> pb::BackendError {
AnkiError::SchemaChange => V::InvalidInput(pb::Empty {}), AnkiError::SchemaChange => V::InvalidInput(pb::Empty {}),
AnkiError::JSONError { info } => V::JsonError(info), AnkiError::JSONError { info } => V::JsonError(info),
AnkiError::ProtoError { info } => V::ProtoError(info), AnkiError::ProtoError { info } => V::ProtoError(info),
AnkiError::NoCardsGenerated => todo!(),
}; };
pb::BackendError { pb::BackendError {

View file

@ -48,9 +48,6 @@ pub enum AnkiError {
#[fail(display = "Operation modifies schema, but schema not marked modified.")] #[fail(display = "Operation modifies schema, but schema not marked modified.")]
SchemaChange, SchemaChange,
#[fail(display = "No cards generated.")]
NoCardsGenerated,
} }
// error helpers // error helpers

View file

@ -142,13 +142,8 @@ impl Collection {
pub(crate) fn add_note_inner(&mut self, ctx: &CardGenContext, note: &mut Note) -> Result<()> { pub(crate) fn add_note_inner(&mut self, ctx: &CardGenContext, note: &mut Note) -> Result<()> {
note.prepare_for_update(&ctx.notetype, ctx.usn)?; 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.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<()> { pub fn update_note(&mut self, note: &mut Note) -> Result<()> {
@ -177,12 +172,7 @@ impl Collection {
#[cfg(test)] #[cfg(test)]
mod test { mod test {
use super::{anki_base91, field_checksum}; use super::{anki_base91, field_checksum};
use crate::{ use crate::{collection::open_test_collection, decks::DeckID, err::Result};
collection::open_test_collection,
decks::DeckID,
err::{AnkiError, Result},
search::SortMode,
};
#[test] #[test]
fn test_base91() { fn test_base91() {
@ -200,35 +190,43 @@ mod test {
} }
#[test] #[test]
fn adding() -> Result<()> { fn adding_cards() -> Result<()> {
let mut col = open_test_collection(); 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(); let mut note = nt.new_note();
assert_eq!(col.add_note(&mut note), Err(AnkiError::NoCardsGenerated)); // if no cards are generated, 1 card is added
note.fields[1] = "foo".into();
assert_eq!(col.add_note(&mut note), Err(AnkiError::NoCardsGenerated));
note.fields[0] = "bar".into();
col.add_note(&mut note).unwrap(); 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!( // nothing changes if the first field is filled
col.search_cards(&format!("nid:{}", note.id), SortMode::NoOrder) note.fields[0] = "test".into();
.unwrap() col.update_note(&mut note).unwrap();
.len(), let existing = col.storage.existing_cards_for_note(note.id)?;
1 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 nt = col.get_notetype_by_name("cloze")?.unwrap();
let mut note = nt.new_note(); let mut note = nt.new_note();
// cloze cards without any cloze deletions are allowed
col.add_note(&mut note).unwrap(); col.add_note(&mut note).unwrap();
let existing = col.storage.existing_cards_for_note(note.id)?; let existing = col.storage.existing_cards_for_note(note.id)?;
assert_eq!(existing.len(), 1); assert_eq!(existing.len(), 1);
assert_eq!(existing[0].ord, 0); assert_eq!(existing[0].ord, 0);
assert_eq!(existing[0].original_deck_id, DeckID(1)); 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(); note.fields[0] = "{{c1::foo}} {{c2::bar}} {{c3::baz}} {{c0::quux}} {{c501::over}}".into();
col.update_note(&mut note)?; col.update_note(&mut note)?;
let existing = col.storage.existing_cards_for_note(note.id)?; let existing = col.storage.existing_cards_for_note(note.id)?;

View file

@ -86,9 +86,20 @@ impl CardGenContext<'_> {
existing: &[AlreadyGeneratedCardInfo], existing: &[AlreadyGeneratedCardInfo],
) -> Vec<CardToGenerate> { ) -> Vec<CardToGenerate> {
let extracted = extract_data_from_existing_cards(existing); 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::Normal => self.new_cards_required_normal(note, &extracted),
NoteTypeKind::Cloze => self.new_cards_required_cloze(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() { for field in note.fields() {
add_cloze_numbers_in_string(field, &mut set); add_cloze_numbers_in_string(field, &mut set);
} }
let cards: Vec<_> = set set.into_iter()
.into_iter()
.filter_map(|cloze_ord| { .filter_map(|cloze_ord| {
let card_ord = cloze_ord.saturating_sub(1).min(499); let card_ord = cloze_ord.saturating_sub(1).min(499);
if extracted.existing_ords.contains(&(card_ord as u32)) { if extracted.existing_ords.contains(&(card_ord as u32)) {
@ -142,17 +152,7 @@ impl CardGenContext<'_> {
}) })
} }
}) })
.collect(); .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
}
} }
} }
@ -200,12 +200,33 @@ pub(crate) fn extract_data_from_existing_cards(
} }
impl Collection { 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( pub(crate) fn generate_cards_for_existing_note(
&mut self, &mut self,
ctx: &CardGenContext, ctx: &CardGenContext,
note: &Note, note: &Note,
) -> Result<()> { ) -> 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); let cards = ctx.new_cards_required(note, &existing);
if cards.is_empty() { if cards.is_empty() {
return Ok(()); return Ok(());
@ -225,7 +246,7 @@ impl Collection {
continue; continue;
} }
let note = self.storage.get_note(nid)?.unwrap(); let note = self.storage.get_note(nid)?.unwrap();
self.generate_cards_for_existing_note(ctx, &note)?; self.generate_cards_for_note(ctx, &note, true)?;
} }
Ok(()) Ok(())

View file

@ -166,12 +166,19 @@ impl NoteType {
self.templates.push(CardTemplate::new(name, qfmt, afmt)); 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 // defaults to 0
self.config.target_deck_id = 1; 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.normalize_names();
self.ensure_names_unique(); self.ensure_names_unique();
self.update_requirements(); self.update_requirements();
Ok(())
} }
pub fn new_note(&self) -> Note { pub fn new_note(&self) -> Note {

View file

@ -240,5 +240,4 @@ mod test {
} }
} }
// fixme: make sure we don't orphan notes // fixme: make sure we don't orphan notes

View file

@ -58,7 +58,7 @@ pub(crate) fn basic(i18n: &I18n) -> NoteType {
fieldref(back), fieldref(back),
), ),
); );
nt.prepare_for_adding(); nt.prepare_for_adding().unwrap();
nt nt
} }
@ -74,7 +74,7 @@ pub(crate) fn basic_typing(i18n: &I18n) -> NoteType {
fieldref(front), fieldref(front),
back back
); );
nt.prepare_for_adding(); nt.prepare_for_adding().unwrap();
nt nt
} }
@ -92,7 +92,7 @@ pub(crate) fn basic_forward_reverse(i18n: &I18n) -> NoteType {
fieldref(front), fieldref(front),
), ),
); );
nt.prepare_for_adding(); nt.prepare_for_adding().unwrap();
nt nt
} }
@ -103,7 +103,7 @@ pub(crate) fn basic_optional_reverse(i18n: &I18n) -> NoteType {
nt.add_field(addrev.as_ref()); nt.add_field(addrev.as_ref());
let tmpl = &mut nt.templates[1].config; let tmpl = &mut nt.templates[1].config;
tmpl.q_format = format!("{{{{#{}}}}}{}{{{{/{}}}}}", addrev, tmpl.q_format, addrev); tmpl.q_format = format!("{{{{#{}}}}}{}{{{{/{}}}}}", addrev, tmpl.q_format, addrev);
nt.prepare_for_adding(); nt.prepare_for_adding().unwrap();
nt nt
} }
@ -124,6 +124,6 @@ pub(crate) fn cloze(i18n: &I18n) -> NoteType {
color: lightblue; color: lightblue;
} }
"; ";
nt.prepare_for_adding(); nt.prepare_for_adding().unwrap();
nt nt
} }