From 45f2502e5b6435c667e3ce0aa57a448b63e16fa2 Mon Sep 17 00:00:00 2001 From: Sam Penny <33956017+sam1penny@users.noreply.github.com> Date: Mon, 22 Aug 2022 02:26:57 +0100 Subject: [PATCH 01/17] fix line break in sync link in the toolbar (#2022) --- qt/aqt/toolbar.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qt/aqt/toolbar.py b/qt/aqt/toolbar.py index c0d0ba320..34b3e652e 100644 --- a/qt/aqt/toolbar.py +++ b/qt/aqt/toolbar.py @@ -144,8 +144,8 @@ class Toolbar: self.link_handlers[label] = self._syncLinkHandler return f""" -{name} - +{name} """ def set_sync_active(self, active: bool) -> None: From 79fbb6c8d8f1ff82c13340e5102cab9ee4f224fd Mon Sep 17 00:00:00 2001 From: RumovZ Date: Wed, 24 Aug 2022 08:04:32 +0200 Subject: [PATCH 02/17] Keep content of unmapped fields when importing (#2023) * Keep content of unmapped fields when importing * Test new behaviour * Fix typo in `canonify_tags_without_resgistering` * Log updated note instead of original one * Revert merging imported tags But keep old note tags if no new ones are provided. --- rslib/src/import_export/text/csv/import.rs | 75 ++++-- rslib/src/import_export/text/import.rs | 297 +++++++++++++-------- rslib/src/import_export/text/mod.rs | 10 +- rslib/src/notes/mod.rs | 22 +- rslib/src/tags/register.rs | 26 +- 5 files changed, 274 insertions(+), 156 deletions(-) diff --git a/rslib/src/import_export/text/csv/import.rs b/rslib/src/import_export/text/csv/import.rs index 6da753812..a38d6166e 100644 --- a/rslib/src/import_export/text/csv/import.rs +++ b/rslib/src/import_export/text/csv/import.rs @@ -56,7 +56,7 @@ impl CsvMetadata { .ok_or_else(|| AnkiError::invalid_input("notetype oneof not set")) } - fn field_source_columns(&self) -> Result>> { + fn field_source_columns(&self) -> Result { Ok(match self.notetype()? { CsvNotetype::GlobalNotetype(global) => global .field_columns @@ -115,8 +115,7 @@ struct ColumnContext { guid_column: Option, deck_column: Option, notetype_column: Option, - /// Source column indices for the fields of a notetype, identified by its - /// name or id as string. The empty string corresponds to the default notetype. + /// Source column indices for the fields of a notetype field_source_columns: FieldSourceColumns, /// How fields are converted to strings. Used for escaping HTML if appropriate. stringify: fn(&str) -> String, @@ -168,22 +167,20 @@ impl ColumnContext { } } - fn gather_tags(&self, record: &csv::StringRecord) -> Vec { - self.tags_column - .and_then(|i| record.get(i - 1)) - .unwrap_or_default() - .split_whitespace() - .filter(|s| !s.is_empty()) - .map(ToString::to_string) - .collect() + fn gather_tags(&self, record: &csv::StringRecord) -> Option> { + self.tags_column.and_then(|i| record.get(i - 1)).map(|s| { + s.split_whitespace() + .filter(|s| !s.is_empty()) + .map(ToString::to_string) + .collect() + }) } - fn gather_note_fields(&self, record: &csv::StringRecord) -> Vec { + fn gather_note_fields(&self, record: &csv::StringRecord) -> Vec> { let stringify = self.stringify; self.field_source_columns .iter() - .map(|opt| opt.and_then(|idx| record.get(idx - 1)).unwrap_or_default()) - .map(stringify) + .map(|opt| opt.and_then(|idx| record.get(idx - 1)).map(stringify)) .collect() } } @@ -253,7 +250,19 @@ mod test { ($metadata:expr, $csv:expr, $expected:expr) => { let notes = import!(&$metadata, $csv); let fields: Vec<_> = notes.into_iter().map(|note| note.fields).collect(); - assert_eq!(fields, $expected); + assert_eq!(fields.len(), $expected.len()); + for (note_fields, note_expected) in fields.iter().zip($expected.iter()) { + assert_field_eq!(note_fields, note_expected); + } + }; + } + + macro_rules! assert_field_eq { + ($fields:expr, $expected:expr) => { + assert_eq!($fields.len(), $expected.len()); + for (field, expected) in $fields.iter().zip($expected.iter()) { + assert_eq!(&field.as_ref().map(String::as_str), expected); + } }; } @@ -283,20 +292,28 @@ mod test { #[test] fn should_allow_missing_columns() { let metadata = CsvMetadata::defaults_for_testing(); - assert_imported_fields!(metadata, "foo\n", &[&["foo", ""]]); + assert_imported_fields!(metadata, "foo\n", [[Some("foo"), None]]); } #[test] fn should_respect_custom_delimiter() { let mut metadata = CsvMetadata::defaults_for_testing(); metadata.set_delimiter(Delimiter::Pipe); - assert_imported_fields!(metadata, "fr,ont|ba,ck\n", &[&["fr,ont", "ba,ck"]]); + assert_imported_fields!( + metadata, + "fr,ont|ba,ck\n", + [[Some("fr,ont"), Some("ba,ck")]] + ); } #[test] fn should_ignore_first_line_starting_with_tags() { let metadata = CsvMetadata::defaults_for_testing(); - assert_imported_fields!(metadata, "tags:foo\nfront,back\n", &[&["front", "back"]]); + assert_imported_fields!( + metadata, + "tags:foo\nfront,back\n", + [[Some("front"), Some("back")]] + ); } #[test] @@ -308,21 +325,29 @@ mod test { id: 1, field_columns: vec![3, 1], })); - assert_imported_fields!(metadata, "front,foo,back\n", &[&["back", "front"]]); + assert_imported_fields!( + metadata, + "front,foo,back\n", + [[Some("back"), Some("front")]] + ); } #[test] fn should_ignore_lines_starting_with_number_sign() { let metadata = CsvMetadata::defaults_for_testing(); - assert_imported_fields!(metadata, "#foo\nfront,back\n#bar\n", &[&["front", "back"]]); + assert_imported_fields!( + metadata, + "#foo\nfront,back\n#bar\n", + [[Some("front"), Some("back")]] + ); } #[test] fn should_escape_html_entities_if_csv_is_html() { let mut metadata = CsvMetadata::defaults_for_testing(); - assert_imported_fields!(metadata, "
\n", &[&["<hr>", ""]]); + assert_imported_fields!(metadata, "
\n", [[Some("<hr>"), None]]); metadata.is_html = true; - assert_imported_fields!(metadata, "
\n", &[&["
", ""]]); + assert_imported_fields!(metadata, "
\n", [[Some("
"), None]]); } #[test] @@ -330,7 +355,7 @@ mod test { let mut metadata = CsvMetadata::defaults_for_testing(); metadata.tags_column = 3; let notes = import!(metadata, "front,back,foo bar\n"); - assert_eq!(notes[0].tags, &["foo", "bar"]); + assert_eq!(notes[0].tags.as_ref().unwrap(), &["foo", "bar"]); } #[test] @@ -347,9 +372,9 @@ mod test { metadata.notetype.replace(CsvNotetype::NotetypeColumn(1)); metadata.column_labels.push("".to_string()); let notes = import!(metadata, "Basic,front,back\nCloze,foo,bar\n"); - assert_eq!(notes[0].fields, &["front", "back"]); + assert_field_eq!(notes[0].fields, [Some("front"), Some("back")]); assert_eq!(notes[0].notetype, NameOrId::Name(String::from("Basic"))); - assert_eq!(notes[1].fields, &["foo", "bar"]); + assert_field_eq!(notes[1].fields, [Some("foo"), Some("bar")]); assert_eq!(notes[1].notetype, NameOrId::Name(String::from("Cloze"))); } } diff --git a/rslib/src/import_export/text/import.rs b/rslib/src/import_export/text/import.rs index f036cc6bc..f51770a29 100644 --- a/rslib/src/import_export/text/import.rs +++ b/rslib/src/import_export/text/import.rs @@ -4,7 +4,6 @@ use std::{ borrow::Cow, collections::{HashMap, HashSet}, - mem, sync::Arc, }; @@ -16,8 +15,9 @@ use crate::{ text::{ DupeResolution, ForeignCard, ForeignData, ForeignNote, ForeignNotetype, ForeignTemplate, }, - ImportProgress, IncrementableProgress, LogNote, NoteLog, + ImportProgress, IncrementableProgress, NoteLog, }, + notes::{field_checksum, normalize_field}, notetype::{CardGenContext, CardTemplate, NoteField, NotetypeConfig}, prelude::*, text::strip_html_preserving_media_filenames, @@ -78,13 +78,13 @@ struct DeckIdsByNameOrId { default: Option, } -struct NoteContext { - /// Prepared and with canonified tags. - note: Note, +struct NoteContext<'a> { + note: ForeignNote, dupes: Vec, - cards: Vec, notetype: Arc, deck_id: DeckId, + global_tags: &'a [String], + updated_tags: &'a [String], } struct Duplicate { @@ -94,8 +94,8 @@ struct Duplicate { } impl Duplicate { - fn new(dupe: Note, original: &Note, first_field_match: bool) -> Self { - let identical = dupe.equal_fields_and_tags(original); + fn new(dupe: Note, original: &ForeignNote, first_field_match: bool) -> Self { + let identical = original.equal_fields_and_tags(&dupe); Self { note: dupe, identical, @@ -190,14 +190,20 @@ impl<'a> Context<'a> { let mut log = NoteLog::new(self.dupe_resolution, notes.len() as u32); for foreign in notes { incrementor.increment()?; - if foreign.first_field_is_empty() { + if foreign.first_field_is_the_empty_string() { log.empty_first_field.push(foreign.into_log_note()); continue; } if let Some(notetype) = self.notetype_for_note(&foreign)? { if let Some(deck_id) = self.deck_ids.get(&foreign.deck) { - let ctx = self.build_note_context(foreign, notetype, deck_id, global_tags)?; - self.import_note(ctx, updated_tags, &mut log)?; + let ctx = self.build_note_context( + foreign, + notetype, + deck_id, + global_tags, + updated_tags, + )?; + self.import_note(ctx, &mut log)?; } else { log.missing_deck.push(foreign.into_log_note()); } @@ -208,41 +214,45 @@ impl<'a> Context<'a> { Ok(log) } - fn build_note_context( + fn build_note_context<'tags>( &mut self, - foreign: ForeignNote, + mut note: ForeignNote, notetype: Arc, deck_id: DeckId, - global_tags: &[String], - ) -> Result { - let (mut note, cards) = foreign.into_native(¬etype, deck_id, self.today, global_tags); - note.prepare_for_update(¬etype, self.normalize_notes)?; - self.col.canonify_note_tags(&mut note, self.usn)?; + global_tags: &'tags [String], + updated_tags: &'tags [String], + ) -> Result> { + self.prepare_foreign_note(&mut note)?; let dupes = self.find_duplicates(¬etype, ¬e)?; - Ok(NoteContext { note, dupes, - cards, notetype, deck_id, + global_tags, + updated_tags, }) } - fn find_duplicates(&self, notetype: &Notetype, note: &Note) -> Result> { - let checksum = note - .checksum - .ok_or_else(|| AnkiError::invalid_input("note unprepared"))?; + fn prepare_foreign_note(&mut self, note: &mut ForeignNote) -> Result<()> { + note.normalize_fields(self.normalize_notes); + self.col.canonify_foreign_tags(note, self.usn) + } + + fn find_duplicates(&self, notetype: &Notetype, note: &ForeignNote) -> Result> { if let Some(nid) = self.existing_guids.get(¬e.guid) { self.get_guid_dupe(*nid, note).map(|dupe| vec![dupe]) - } else if let Some(nids) = self.existing_checksums.get(&(notetype.id, checksum)) { + } else if let Some(nids) = note + .checksum() + .and_then(|csum| self.existing_checksums.get(&(notetype.id, csum))) + { self.get_first_field_dupes(note, nids) } else { Ok(Vec::new()) } } - fn get_guid_dupe(&self, nid: NoteId, original: &Note) -> Result { + fn get_guid_dupe(&self, nid: NoteId, original: &ForeignNote) -> Result { self.col .storage .get_note(nid)? @@ -250,7 +260,7 @@ impl<'a> Context<'a> { .map(|dupe| Duplicate::new(dupe, original, false)) } - fn get_first_field_dupes(&self, note: &Note, nids: &[NoteId]) -> Result> { + fn get_first_field_dupes(&self, note: &ForeignNote, nids: &[NoteId]) -> Result> { Ok(self .col .get_full_duplicates(note, nids)? @@ -259,26 +269,36 @@ impl<'a> Context<'a> { .collect()) } - fn import_note( - &mut self, - ctx: NoteContext, - updated_tags: &[String], - log: &mut NoteLog, - ) -> Result<()> { + fn import_note(&mut self, ctx: NoteContext, log: &mut NoteLog) -> Result<()> { match self.dupe_resolution { - _ if ctx.dupes.is_empty() => self.add_note(ctx, &mut log.new)?, - DupeResolution::Add => self.add_note(ctx, &mut log.first_field_match)?, - DupeResolution::Update => self.update_with_note(ctx, updated_tags, log)?, + _ if ctx.dupes.is_empty() => self.add_note(ctx, log, false)?, + DupeResolution::Add => self.add_note(ctx, log, true)?, + DupeResolution::Update => self.update_with_note(ctx, log)?, DupeResolution::Ignore => log.first_field_match.push(ctx.note.into_log_note()), } Ok(()) } - fn add_note(&mut self, mut ctx: NoteContext, log_queue: &mut Vec) -> Result<()> { - ctx.note.usn = self.usn; - self.col.add_note_only_undoable(&mut ctx.note)?; - self.add_cards(&mut ctx.cards, &ctx.note, ctx.deck_id, ctx.notetype)?; - log_queue.push(ctx.note.into_log_note()); + fn add_note(&mut self, ctx: NoteContext, log: &mut NoteLog, dupe: bool) -> Result<()> { + if !ctx.note.first_field_is_unempty() { + log.empty_first_field.push(ctx.note.into_log_note()); + return Ok(()); + } + + let mut note = Note::new(&ctx.notetype); + let mut cards = ctx + .note + .into_native(&mut note, ctx.deck_id, self.today, ctx.global_tags); + self.prepare_note(&mut note, &ctx.notetype)?; + self.col.add_note_only_undoable(&mut note)?; + self.add_cards(&mut cards, ¬e, ctx.deck_id, ctx.notetype)?; + + if dupe { + log.first_field_match.push(note.into_log_note()); + } else { + log.new.push(note.into_log_note()); + } + Ok(()) } @@ -293,63 +313,46 @@ impl<'a> Context<'a> { self.generate_missing_cards(notetype, deck_id, note) } - fn update_with_note( - &mut self, - mut ctx: NoteContext, - updated_tags: &[String], - log: &mut NoteLog, - ) -> Result<()> { - self.prepare_note_for_update(&mut ctx.note, updated_tags)?; - for dupe in mem::take(&mut ctx.dupes) { - self.maybe_update_dupe(dupe, &mut ctx, log)?; + fn update_with_note(&mut self, ctx: NoteContext, log: &mut NoteLog) -> Result<()> { + for dupe in ctx.dupes { + if dupe.note.notetype_id != ctx.notetype.id { + log.conflicting.push(dupe.note.into_log_note()); + continue; + } + + let mut note = dupe.note.clone(); + let mut cards = ctx.note.clone().into_native( + &mut note, + ctx.deck_id, + self.today, + ctx.global_tags.iter().chain(ctx.updated_tags.iter()), + ); + + if !dupe.identical { + self.prepare_note(&mut note, &ctx.notetype)?; + self.col.update_note_undoable(¬e, &dupe.note)?; + } + self.add_cards(&mut cards, ¬e, ctx.deck_id, ctx.notetype.clone())?; + + if dupe.identical { + log.duplicate.push(dupe.note.into_log_note()); + } else if dupe.first_field_match { + log.first_field_match.push(note.into_log_note()); + } else { + log.updated.push(note.into_log_note()); + } } + Ok(()) } - fn prepare_note_for_update(&mut self, note: &mut Note, updated_tags: &[String]) -> Result<()> { - if !updated_tags.is_empty() { - note.tags.extend(updated_tags.iter().cloned()); - self.col.canonify_note_tags(note, self.usn)?; - } + fn prepare_note(&mut self, note: &mut Note, notetype: &Notetype) -> Result<()> { + note.prepare_for_update(notetype, self.normalize_notes)?; + self.col.canonify_note_tags(note, self.usn)?; note.set_modified(self.usn); Ok(()) } - fn maybe_update_dupe( - &mut self, - dupe: Duplicate, - ctx: &mut NoteContext, - log: &mut NoteLog, - ) -> Result<()> { - if dupe.note.notetype_id != ctx.notetype.id { - log.conflicting.push(dupe.note.into_log_note()); - return Ok(()); - } - if dupe.identical { - log.duplicate.push(dupe.note.into_log_note()); - } else { - self.update_dupe(dupe, ctx, log)?; - } - self.add_cards(&mut ctx.cards, &ctx.note, ctx.deck_id, ctx.notetype.clone()) - } - - fn update_dupe( - &mut self, - dupe: Duplicate, - ctx: &mut NoteContext, - log: &mut NoteLog, - ) -> Result<()> { - ctx.note.id = dupe.note.id; - ctx.note.guid = dupe.note.guid.clone(); - self.col.update_note_undoable(&ctx.note, &dupe.note)?; - if dupe.first_field_match { - log.first_field_match.push(dupe.note.into_log_note()); - } else { - log.updated.push(dupe.note.into_log_note()); - } - Ok(()) - } - fn import_cards(&mut self, cards: &mut [Card], note_id: NoteId) -> Result<()> { for card in cards { card.note_id = note_id; @@ -397,8 +400,18 @@ impl Collection { } } - fn get_full_duplicates(&self, note: &Note, dupe_ids: &[NoteId]) -> Result> { - let first_field = note.first_field_stripped(); + fn canonify_foreign_tags(&mut self, note: &mut ForeignNote, usn: Usn) -> Result<()> { + if let Some(tags) = note.tags.take() { + note.tags + .replace(self.canonify_tags_without_registering(tags, usn)?); + } + Ok(()) + } + + fn get_full_duplicates(&self, note: &ForeignNote, dupe_ids: &[NoteId]) -> Result> { + let first_field = note + .first_field_stripped() + .ok_or_else(|| AnkiError::invalid_input("no first field"))?; dupe_ids .iter() .filter_map(|&dupe_id| self.storage.get_note(dupe_id).transpose()) @@ -411,35 +424,72 @@ impl Collection { } impl ForeignNote { - fn into_native( + /// Updates a native note with the foreign data and returns its new cards. + fn into_native<'tags>( self, - notetype: &Notetype, + note: &mut Note, deck_id: DeckId, today: u32, - extra_tags: &[String], - ) -> (Note, Vec) { + extra_tags: impl IntoIterator, + ) -> Vec { // TODO: Handle new and learning cards - let mut note = Note::new(notetype); if !self.guid.is_empty() { note.guid = self.guid; } - note.tags = self.tags; - note.tags.extend(extra_tags.iter().cloned()); + if let Some(tags) = self.tags { + note.tags = tags; + } + note.tags.extend(extra_tags.into_iter().cloned()); note.fields_mut() .iter_mut() .zip(self.fields.into_iter()) - .for_each(|(field, value)| *field = value); - let cards = self - .cards + .for_each(|(field, new)| { + if let Some(s) = new { + *field = s; + } + }); + self.cards .into_iter() .enumerate() .map(|(idx, c)| c.into_native(NoteId(0), idx as u16, deck_id, today)) - .collect(); - (note, cards) + .collect() } - fn first_field_is_empty(&self) -> bool { - self.fields.get(0).map(String::is_empty).unwrap_or(true) + fn first_field_is_the_empty_string(&self) -> bool { + matches!(self.fields.get(0), Some(Some(s)) if s.is_empty()) + } + + fn first_field_is_unempty(&self) -> bool { + matches!(self.fields.get(0), Some(Some(s)) if !s.is_empty()) + } + + fn normalize_fields(&mut self, normalize_text: bool) { + for field in self.fields.iter_mut().flatten() { + normalize_field(field, normalize_text); + } + } + + /// Expects normalized form. + fn equal_fields_and_tags(&self, other: &Note) -> bool { + self.tags.as_ref().map_or(true, |tags| *tags == other.tags) + && self + .fields + .iter() + .zip(other.fields()) + .all(|(opt, field)| opt.as_ref().map(|s| s == field).unwrap_or(true)) + } + + fn first_field_stripped(&self) -> Option> { + self.fields + .get(0) + .and_then(|s| s.as_ref()) + .map(|field| strip_html_preserving_media_filenames(field.as_str())) + } + + /// If the first field is set, returns its checksum. Field is expected to be normalized. + fn checksum(&self) -> Option { + self.first_field_stripped() + .map(|field| field_checksum(&field)) } } @@ -493,12 +543,6 @@ impl ForeignTemplate { } } -impl Note { - fn equal_fields_and_tags(&self, other: &Self) -> bool { - self.fields() == other.fields() && self.tags == other.tags - } -} - #[cfg(test)] mod test { use super::*; @@ -515,7 +559,7 @@ mod test { fn add_note(&mut self, fields: &[&str]) { self.notes.push(ForeignNote { - fields: fields.iter().map(ToString::to_string).collect(), + fields: fields.iter().map(ToString::to_string).map(Some).collect(), ..Default::default() }); } @@ -543,7 +587,7 @@ mod test { data.clone().import(&mut col, |_, _| true).unwrap(); assert_eq!(col.storage.notes_table_len(), 1); - data.notes[0].fields[1] = "new".to_string(); + data.notes[0].fields[1].replace("new".to_string()); data.import(&mut col, |_, _| true).unwrap(); let notes = col.storage.get_all_notes(); assert_eq!(notes.len(), 1); @@ -560,11 +604,30 @@ mod test { data.clone().import(&mut col, |_, _| true).unwrap(); assert_eq!(col.storage.notes_table_len(), 1); - data.notes[0].fields[1] = "new".to_string(); + data.notes[0].fields[1].replace("new".to_string()); data.import(&mut col, |_, _| true).unwrap(); assert_eq!(col.storage.get_all_notes()[0].fields()[1], "new"); } + #[test] + fn should_keep_old_field_content_if_no_new_one_is_supplied() { + let mut col = open_test_collection(); + let mut data = ForeignData::with_defaults(); + data.add_note(&["same", "unchanged"]); + data.add_note(&["same", "unchanged"]); + data.dupe_resolution = DupeResolution::Update; + + data.clone().import(&mut col, |_, _| true).unwrap(); + assert_eq!(col.storage.notes_table_len(), 2); + + data.notes[0].fields[1] = None; + data.notes[1].fields.pop(); + data.import(&mut col, |_, _| true).unwrap(); + let notes = col.storage.get_all_notes(); + assert_eq!(notes[0].fields(), &["same", "unchanged"]); + assert_eq!(notes[0].fields(), &["same", "unchanged"]); + } + #[test] fn should_recognize_normalized_duplicate_only_if_normalization_is_enabled() { let mut col = open_test_collection(); @@ -589,7 +652,7 @@ mod test { let mut col = open_test_collection(); let mut data = ForeignData::with_defaults(); data.add_note(&["foo"]); - data.notes[0].tags = vec![String::from("bar")]; + data.notes[0].tags.replace(vec![String::from("bar")]); data.global_tags = vec![String::from("baz")]; data.import(&mut col, |_, _| true).unwrap(); @@ -601,7 +664,7 @@ mod test { let mut col = open_test_collection(); let mut data = ForeignData::with_defaults(); data.add_note(&["foo"]); - data.notes[0].tags = vec![String::from("bar")]; + data.notes[0].tags.replace(vec![String::from("bar")]); data.global_tags = vec![String::from("baz")]; data.import(&mut col, |_, _| true).unwrap(); diff --git a/rslib/src/import_export/text/mod.rs b/rslib/src/import_export/text/mod.rs index b3cefbdee..e976dca5a 100644 --- a/rslib/src/import_export/text/mod.rs +++ b/rslib/src/import_export/text/mod.rs @@ -26,8 +26,8 @@ pub struct ForeignData { #[serde(default)] pub struct ForeignNote { guid: String, - fields: Vec, - tags: Vec, + fields: Vec>, + tags: Option>, notetype: NameOrId, deck: NameOrId, cards: Vec, @@ -82,7 +82,11 @@ impl ForeignNote { pub(crate) fn into_log_note(self) -> LogNote { LogNote { id: None, - fields: self.fields, + fields: self + .fields + .into_iter() + .map(Option::unwrap_or_default) + .collect(), } } } diff --git a/rslib/src/notes/mod.rs b/rslib/src/notes/mod.rs index f6a1b726b..ae198d5ac 100644 --- a/rslib/src/notes/mod.rs +++ b/rslib/src/notes/mod.rs @@ -186,16 +186,8 @@ impl Note { ))); } - for field in &mut self.fields { - if field.contains(invalid_char_for_field) { - *field = field.replace(invalid_char_for_field, ""); - } - } - - if normalize_text { - for field in &mut self.fields { - ensure_string_in_nfc(field); - } + for field in self.fields_mut() { + normalize_field(field, normalize_text); } let field1_nohtml = strip_html_preserving_media_filenames(&self.fields()[0]); @@ -265,6 +257,16 @@ impl Note { } } +/// Remove invalid characters and optionally ensure nfc normalization. +pub(crate) fn normalize_field(field: &mut String, normalize_text: bool) { + if field.contains(invalid_char_for_field) { + *field = field.replace(invalid_char_for_field, ""); + } + if normalize_text { + ensure_string_in_nfc(field); + } +} + impl From for pb::Note { fn from(n: Note) -> Self { pb::Note { diff --git a/rslib/src/tags/register.rs b/rslib/src/tags/register.rs index b2e902224..bb5cab145 100644 --- a/rslib/src/tags/register.rs +++ b/rslib/src/tags/register.rs @@ -17,6 +17,26 @@ impl Collection { &mut self, tags: Vec, usn: Usn, + ) -> Result<(Vec, bool)> { + self.canonify_tags_inner(tags, usn, true) + } + + pub(crate) fn canonify_tags_without_registering( + &mut self, + tags: Vec, + usn: Usn, + ) -> Result> { + self.canonify_tags_inner(tags, usn, false) + .map(|(tags, _)| tags) + } + + /// Like [canonify_tags()], but doesn't save new tags. As a consequence, new + /// parents are not canonified. + fn canonify_tags_inner( + &mut self, + tags: Vec, + usn: Usn, + register: bool, ) -> Result<(Vec, bool)> { let mut seen = HashSet::new(); let mut added = false; @@ -24,7 +44,11 @@ impl Collection { let tags: Vec<_> = tags.iter().flat_map(|t| split_tags(t)).collect(); for tag in tags { let mut tag = Tag::new(tag.to_string(), usn); - added |= self.register_tag(&mut tag)?; + if register { + added |= self.register_tag(&mut tag)?; + } else { + self.prepare_tag_for_registering(&mut tag)?; + } seen.insert(UniCase::new(tag.name)); } From 825c88b6e8df51caa1ff8b51ebfd7f108e6106c8 Mon Sep 17 00:00:00 2001 From: Aristotelis Date: Wed, 24 Aug 2022 08:07:44 +0200 Subject: [PATCH 03/17] Make all Anki-native exceptions inherit from the same base class (#2028) * Make all Anki-native exceptions inherit from same base class Allows add-ons to easily catch all Anki-native exceptions without being coupled to the currently implemented exceptions. * Satisfy pylint --- pylib/anki/errors.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/pylib/anki/errors.py b/pylib/anki/errors.py index 35fb6d013..b1ca0c7c6 100644 --- a/pylib/anki/errors.py +++ b/pylib/anki/errors.py @@ -10,7 +10,22 @@ if TYPE_CHECKING: import anki.collection -class LocalizedError(Exception): +class AnkiException(Exception): + """ + General Anki exception that all custom exceptions raised by Anki should + inherit from. Allows add-ons to easily identify Anki-native exceptions. + + When inheriting from a Python built-in exception other than `Exception`, + please supply `AnkiException` as an additional inheritance: + + ``` + class MyNewAnkiException(ValueError, AnkiException): + pass + ``` + """ + + +class LocalizedError(AnkiException): "An error with a localized description." def __init__(self, localized: str) -> None: @@ -29,7 +44,7 @@ class DocumentedError(LocalizedError): super().__init__(localized) -class Interrupted(Exception): +class Interrupted(AnkiException): pass @@ -68,7 +83,7 @@ class TemplateError(LocalizedError): pass -class NotFoundError(Exception): +class NotFoundError(AnkiException): pass @@ -76,11 +91,11 @@ class DeletedError(LocalizedError): pass -class ExistsError(Exception): +class ExistsError(AnkiException): pass -class UndoEmpty(Exception): +class UndoEmpty(AnkiException): pass @@ -96,7 +111,7 @@ class SearchError(LocalizedError): pass -class AbortSchemaModification(Exception): +class AbortSchemaModification(AnkiException): pass From 46746e3e34656f3c1e997c8c8d42e528120e9a9f Mon Sep 17 00:00:00 2001 From: Aristotelis Date: Wed, 24 Aug 2022 08:08:58 +0200 Subject: [PATCH 04/17] Move VS Code settings to .vscode.dist and update docs (#2029) --- .gitignore | 1 + {.vscode => .vscode.dist}/extensions.json | 0 {.vscode => .vscode.dist}/settings.json | 0 docs/editing.md | 17 +++++++++++++++-- 4 files changed, 16 insertions(+), 2 deletions(-) rename {.vscode => .vscode.dist}/extensions.json (100%) rename {.vscode => .vscode.dist}/settings.json (100%) diff --git a/.gitignore b/.gitignore index 350f526f2..d9b0a7aef 100644 --- a/.gitignore +++ b/.gitignore @@ -6,5 +6,6 @@ target .dmypy.json node_modules /.idea/ +/.vscode/ /.bazel /windows.bazelrc diff --git a/.vscode/extensions.json b/.vscode.dist/extensions.json similarity index 100% rename from .vscode/extensions.json rename to .vscode.dist/extensions.json diff --git a/.vscode/settings.json b/.vscode.dist/settings.json similarity index 100% rename from .vscode/settings.json rename to .vscode.dist/settings.json diff --git a/docs/editing.md b/docs/editing.md index 6000dac72..86656f6f4 100644 --- a/docs/editing.md +++ b/docs/editing.md @@ -1,8 +1,7 @@ # Editing/IDEs Visual Studio Code is recommended, since it provides decent support for all the languages -Anki uses. If you open the root of this repo in VS Code, it will suggest some extensions -for you to install. +Anki uses. To set up the recommended workspace settings for VS Code, please see below. For editing Python, PyCharm/IntelliJ's type checking/completion is a bit nicer than VS Code, but VS Code has improved considerably in a short span of time. @@ -36,6 +35,20 @@ Code completion partly depends on files that are generated as part of the regular build process, so for things to work correctly, use './run' or 'tools/build' prior to using code completion. +## Visual Studio Code + +### Setting up Recommended Workspace Settings + +To start off with some default workspace settings that are optimized for Anki development, please head to the project root and then run: + +``` +cp -r .vscode.dist .vscode +``` + +### Installing Recommended Extensions + +Once the workspace settings are set up, open the root of the repo in VS Code to see and install a number of recommended extensions. + ## PyCharm/IntelliJ If you decide to use PyCharm instead of VS Code, there are somethings to be aware of. From 89c4441837f6cc6c3361eb81c2abd1ea4d4250e3 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Wed, 24 Aug 2022 18:37:58 +1000 Subject: [PATCH 05/17] Fix scheduler change not reflected after normal sync --- qt/aqt/sync.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qt/aqt/sync.py b/qt/aqt/sync.py index 2ff0de237..94f95b117 100644 --- a/qt/aqt/sync.py +++ b/qt/aqt/sync.py @@ -95,6 +95,8 @@ def sync_collection(mw: aqt.main.AnkiQt, on_done: Callable[[], None]) -> None: def on_future_done(fut: Future) -> None: mw.col.db.begin() + # scheduler version may have changed + mw.col._load_scheduler() timer.stop() try: out: SyncOutput = fut.result() From 4a884d379fe4a27c3680f0c7c8684f88e5736d56 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Thu, 25 Aug 2022 18:04:37 +1000 Subject: [PATCH 06/17] Fix build failing on macOS I suspect the PyQt maintainer uploaded a new wheel and yanked the old one, which made pip fall back on a source install which failed. If that's the case, he really should have used a new version number, as this makes building/bisecting older releases more cumbersome. --- python/pyqt/6/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/pyqt/6/requirements.txt b/python/pyqt/6/requirements.txt index 00e88ef72..8d4021591 100644 --- a/python/pyqt/6/requirements.txt +++ b/python/pyqt/6/requirements.txt @@ -18,8 +18,8 @@ pyqt6-sip==13.4.0 \ --hash=sha256:2694ae67811cefb6ea3ee0e9995755b45e4952f4dcadec8c04300fd828f91c75 \ --hash=sha256:3486914137f5336cff6e10a5e9d52c1e60ff883473938b45f267f794daeacb2f \ --hash=sha256:3ac7e0800180202dcc0c7035ff88c2a6f4a0f5acb20c4a19f71d807d0f7857b7 \ + --hash=sha256:3de18c4a32f717a351d560a39f528af24077f5135aacfa8890a2f2d79f0633da \ --hash=sha256:6d87a3ee5872d7511b76957d68a32109352caf3b7a42a01d9ee20032b350d979 \ - --hash=sha256:7b9bbb5fb880440a3a8e7fa3dff70473aa1128aaf7dc9fb6e30512eed4fd38f6 \ --hash=sha256:802b0cfed19900183220c46895c2635f0dd062f2d275a25506423f911ef74db4 \ --hash=sha256:83b446d247a92d119d507dbc94fc1f47389d8118a5b6232a2859951157319a30 \ --hash=sha256:9c5231536e6153071b22175e46e368045fd08d772a90d772a0977d1166c7822c \ From 966d7f3760836830d5d8471574606efc38f2c326 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Tue, 30 Aug 2022 21:52:22 +1000 Subject: [PATCH 07/17] Experimentally remove webview recycling It was originally introduced for WebKit, and may no longer be pertinent for Chromium.WebEngine. https://forums.ankiweb.net/t/anki-glitch-showing-previous-card-answer-instead-of-new-card/12482/13 --- qt/aqt/reviewer.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qt/aqt/reviewer.py b/qt/aqt/reviewer.py index 722dd70d4..ca984121a 100644 --- a/qt/aqt/reviewer.py +++ b/qt/aqt/reviewer.py @@ -230,8 +230,7 @@ class Reviewer: self.mw.moveToState("overview") return - if self._reps is None or self._reps % 100 == 0: - # we recycle the webview periodically so webkit can free memory + if self._reps is None: self._initWeb() self._showQuestion() From 28eae0667a048f2df5ccd9fd635bf1f467777f15 Mon Sep 17 00:00:00 2001 From: BlueGreenMagick Date: Wed, 31 Aug 2022 17:35:01 +0900 Subject: [PATCH 08/17] fix wrong rich-text-widget offset (#2033) .rich-text-input { padding: 6px; } messed up image & mathjax overlay positioning because parent padding isn't added to absolute positioning px --- ts/editor/FieldDescription.svelte | 3 --- ts/editor/rich-text-input/RichTextInput.svelte | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/ts/editor/FieldDescription.svelte b/ts/editor/FieldDescription.svelte index 41f2cd105..40ba2927c 100644 --- a/ts/editor/FieldDescription.svelte +++ b/ts/editor/FieldDescription.svelte @@ -40,9 +40,6 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html opacity: 0.4; pointer-events: none; - /* same as in ContentEditable */ - padding: 6px; - /* stay a on single line */ overflow-x: hidden; white-space: nowrap; diff --git a/ts/editor/rich-text-input/RichTextInput.svelte b/ts/editor/rich-text-input/RichTextInput.svelte index c04813c95..47be4117a 100644 --- a/ts/editor/rich-text-input/RichTextInput.svelte +++ b/ts/editor/rich-text-input/RichTextInput.svelte @@ -244,7 +244,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html diff --git a/ts/editor/EditorField.svelte b/ts/editor/EditorField.svelte index bd3f1983d..9f903c824 100644 --- a/ts/editor/EditorField.svelte +++ b/ts/editor/EditorField.svelte @@ -14,6 +14,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html direction: "ltr" | "rtl"; plainText: boolean; description: string; + collapsed: boolean; } export interface EditorFieldAPI { @@ -54,6 +55,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html export let content: Writable; export let field: FieldData; export let collapsed = false; + export let flipInputs = false; const directionStore = writable<"ltr" | "rtl">(); setContext(directionKey, directionStore); @@ -101,7 +103,13 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html fontSize={field.fontSize} api={editingArea} > - + {#if flipInputs} + + + {:else} + + + {/if} diff --git a/ts/editor/NoteEditor.svelte b/ts/editor/NoteEditor.svelte index 9f942f3ba..7aaa1d01f 100644 --- a/ts/editor/NoteEditor.svelte +++ b/ts/editor/NoteEditor.svelte @@ -66,6 +66,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html import PlainTextInput from "./plain-text-input"; import PlainTextBadge from "./PlainTextBadge.svelte"; import RichTextInput, { editingInputIsRichText } from "./rich-text-input"; + import RichTextBadge from "./RichTextBadge.svelte"; function quoteFontFamily(fontFamily: string): string { // generic families (e.g. sans-serif) must not be quoted @@ -113,13 +114,19 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html fieldNames = newFieldNames; } - let plainTexts: boolean[] = []; + let fieldsCollapsed: boolean[] = []; + export function setCollapsed(fs: boolean[]): void { + fieldsCollapsed = fs; + } + let richTextsHidden: boolean[] = []; let plainTextsHidden: boolean[] = []; + let plainTextDefaults: boolean[] = []; export function setPlainTexts(fs: boolean[]): void { - richTextsHidden = plainTexts = fs; + richTextsHidden = fs; plainTextsHidden = Array.from(fs, (v) => !v); + plainTextDefaults = [...richTextsHidden]; } function setMathjaxEnabled(enabled: boolean): void { @@ -132,13 +139,11 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html } let fonts: [string, number, boolean][] = []; - let fieldsCollapsed: boolean[] = []; const fields = clearableArray(); export function setFonts(fs: [string, number, boolean][]): void { fonts = fs; - fieldsCollapsed = fonts.map((_, index) => fieldsCollapsed[index] ?? false); } export function focusField(index: number | null): void { @@ -186,11 +191,12 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html $: fieldsData = fieldNames.map((name, index) => ({ name, - plainText: plainTexts[index], + plainText: plainTextDefaults[index], description: fieldDescriptions[index], fontFamily: quoteFontFamily(fonts[index][0]), fontSize: fonts[index][1], direction: fonts[index][2] ? "rtl" : "ltr", + collapsed: fieldsCollapsed[index], })) as FieldData[]; function saveTags({ detail }: CustomEvent): void { @@ -241,6 +247,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html import { mathjaxConfig } from "../editable/mathjax-element"; import { wrapInternal } from "../lib/wrap"; + import { refocusInput } from "./helpers"; import * as oldEditorAdapter from "./old-editor-adapter"; onMount(() => { @@ -256,6 +263,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html Object.assign(globalThis, { setFields, + setCollapsed, setPlainTexts, setDescriptions, setFonts, @@ -329,6 +337,7 @@ the AddCards dialog) should be implemented in the user of this component. { $focusedField = fields[index]; @@ -357,11 +366,16 @@ the AddCards dialog) should be implemented in the user of this component. on:toggle={async () => { fieldsCollapsed[index] = !fieldsCollapsed[index]; + const defaultInput = !plainTextDefaults[index] + ? richTextInputs[index] + : plainTextInputs[index]; + if (!fieldsCollapsed[index]) { - await tick(); - richTextInputs[index].api.refocus(); - } else { + refocusInput(defaultInput.api); + } else if (!plainTextDefaults[index]) { plainTextsHidden[index] = true; + } else { + richTextsHidden[index] = true; } }} > @@ -374,21 +388,41 @@ the AddCards dialog) should be implemented in the user of this component. {#if cols[index] === "dupe"} {/if} - { - plainTextsHidden[index] = - !plainTextsHidden[index]; + {#if plainTextDefaults[index]} + { + richTextsHidden[index] = + !richTextsHidden[index]; - if (!plainTextsHidden[index]) { - await tick(); - plainTextInputs[index].api.refocus(); - } - }} - /> + if (!richTextsHidden[index]) { + refocusInput( + richTextInputs[index].api, + ); + } + }} + /> + {:else} + { + plainTextsHidden[index] = + !plainTextsHidden[index]; + + if (!plainTextsHidden[index]) { + refocusInput( + plainTextInputs[index].api, + ); + } + }} + /> + {/if} - - + + { saveFieldNow(); $focusedInput = null; @@ -416,10 +450,13 @@ the AddCards dialog) should be implemented in the user of this component. - - + + + { saveFieldNow(); $focusedInput = null; diff --git a/ts/editor/RichTextBadge.svelte b/ts/editor/RichTextBadge.svelte new file mode 100644 index 000000000..180925395 --- /dev/null +++ b/ts/editor/RichTextBadge.svelte @@ -0,0 +1,59 @@ + + + + + {@html richTextIcon} + + + diff --git a/ts/editor/helpers.ts b/ts/editor/helpers.ts index 98fe4f892..53aeee6d4 100644 --- a/ts/editor/helpers.ts +++ b/ts/editor/helpers.ts @@ -1,6 +1,9 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html +import type { PlainTextInputAPI } from "./plain-text-input"; +import type { RichTextInputAPI } from "./rich-text-input"; + function isFontElement(element: Element): element is HTMLFontElement { return element.tagName === "FONT"; } @@ -19,3 +22,15 @@ export function withFontColor( return false; } + +/*** + * Required for field inputs wrapped in Collapsible + */ +export async function refocusInput( + api: RichTextInputAPI | PlainTextInputAPI, +): Promise { + do { + await new Promise(window.requestAnimationFrame); + } while (!api.focusable); + api.refocus(); +} diff --git a/ts/editor/plain-text-input/PlainTextInput.svelte b/ts/editor/plain-text-input/PlainTextInput.svelte index 67d4f8049..d3479f418 100644 --- a/ts/editor/plain-text-input/PlainTextInput.svelte +++ b/ts/editor/plain-text-input/PlainTextInput.svelte @@ -39,7 +39,9 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html import removeProhibitedTags from "./remove-prohibited"; import { storedToUndecorated, undecoratedToStored } from "./transform"; + export let isDefault: boolean; export let hidden: boolean; + export let richTextHidden: boolean; const configuration = { mode: htmlanki, @@ -143,6 +145,8 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
($focusedInput = api)} > .plain-text-input { - overflow-x: hidden; + overflow: hidden; + + border-top: 1px solid var(--border); + border-radius: 0 0 5px 5px; + + &.is-default { + border-top: none; + border-bottom: 1px solid var(--border); + border-radius: 5px 5px 0 0; + } + &.alone { + border: none; + border-radius: 5px; + } :global(.CodeMirror) { - border-radius: 0 0 5px 5px; - border-top: 1px solid var(--border); background: var(--code-bg); } :global(.CodeMirror-lines) { From 52b4e3ad1675cd30261b1dee8bf32773d9f4ec2c Mon Sep 17 00:00:00 2001 From: RumovZ Date: Thu, 1 Sep 2022 14:01:14 +0200 Subject: [PATCH 13/17] Document Protocol Buffers used in Anki (#2042) * Document Protocol Buffers used in Anki * Fix typos * Correct notes about Rust crate --- docs/protobuf.md | 122 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 docs/protobuf.md diff --git a/docs/protobuf.md b/docs/protobuf.md new file mode 100644 index 000000000..240b73beb --- /dev/null +++ b/docs/protobuf.md @@ -0,0 +1,122 @@ +# Protocol Buffers + +Anki uses [different implementations of Protocol Buffers](./architecture.md#protobuf) +and each has its own pecularities. This document highlights some aspects relevant +to Anki and hopefully helps to avoid some common pitfalls. + +For information about Protobuf's types and syntax, please see the official [language guide](https://developers.google.com/protocol-buffers/docs/proto3). + +## General Notes + +### Names + +Generated code follows the naming conventions of the targeted language. So to access +the message field `foo_bar` you need to use `fooBar` in Typescript and the +namespace created by the message `FooBar` is called `foo_bar` in Rust. + +### Optional Values + +In Python and Typescript, unset optional values will contain the type's default +value rather than `None`, `null` or `undefined`. Here's an example: + +```protobuf +message Foo { + optional string name = 1; + optional int32 number = 2; +} +``` + +```python +message = Foo() +assert message.number == 0 +assert message name == "" +``` + +In Python, we can use the message's `HasField()` method to check whether a field is +actually set: + +```python +message = Foo(name="") +assert message.HasField("name") +assert not message.HasField("number") +``` + +In Typescript, this is even less ergonomic and it can be easier to avoid using +the default values in active fields. E.g. the `CsvMetadata` message uses 1-based +indices instead of optional 0-based ones to avoid ambiguity when an index is `0`. + +### Oneofs + +All fields in a oneof are implicitly optional, so the caveats [above](#optional-values) +apply just as much to a message like this: + +```protobuf +message Foo { + oneof bar { + string name = 1; + int32 number = 2; + } +} +``` + +In addition to `HasField()`, `WhichOneof()` can be used to get the name of the set +field: + +```python +message = Foo(name="") +assert message.WhichOneof("bar") == "name" +``` + +### Backwards Compatibility + +The official [language guide](https://developers.google.com/protocol-buffers/docs/proto3) +makes a lot of notes about backwards compatibility, but as Anki usually doesn't +use Protobuf to communicate between different clients, things like shuffling around +field numbers are usually not a concern. + +However, there are some messages, like `Deck`, which get stored in the database. +If these are modified in an incompatible way, this can lead to serious issues if +clients with a different protocol try to read them. Such modifications are only +safe to make as part of a schema upgrade, because schema 11 (the targeted schema +when choosing _Downgrade_), does not make use of Protobuf messages. + +### Field Numbers + +Field numbers larger than 15 need an additional byte to encode, so `repeated` fields +should preferrably be assigned a number between 1 and 15. If a message contains +`reserved` fields, this is usually to accomodate potential future `repeated` fields. + +## Implementation-Specific Notes + +### Python + +Protobuf has an official Python implementation with an extensive [reference](https://developers.google.com/protocol-buffers/docs/reference/python-generated). + +- Every message used in aqt or pylib must be added to the respective `.pylintrc` + to avoid failing type checks. The unqualified protobuf message's name must be + used, not an alias from `collection.py` for example. This should be taken into + account when choosing a message name in order to prevent skipping typechecking + a Python class of the same name. + +### Typescript + +Anki uses [protobuf.js](https://protobufjs.github.io/protobuf.js/), which offers +some documentation. + +- If using a message `Foo` as a type, make sure not to use the generated interface + `IFoo` instead. Their definitions are very similar, but the interface requires + null checks for every field. + +### Rust + +Anki uses the [prost crate](https://docs.rs/prost/latest/prost/). +Its documentation has some useful hints, but for working with the generated code, +there is a better option: From within `anki/rslib` run `cargo doc --open --document-private-items`. +Inside the `pb` module you will find all generated Rust types and their implementations. + +- Given an enum field `Foo foo = 1;`, `message.foo` is an `i32`. Use the accessor + `message.foo()` instead to avoid having to manually convert to a `Foo`. +- Protobuf does not guarantee any oneof field to be set or an enum field to contain + a valid variant, so the Rust code needs to deal with a lot of `Option`s. As we + don't expect other parts of Anki to send invalid messages, using an `InvalidInput` + error or `unwrap_or_default()` is usually fine. From 53293284c9346c296bb24becae69e67e49adcb4c Mon Sep 17 00:00:00 2001 From: Matthias Metelka <62722460+kleinerpirat@users.noreply.github.com> Date: Thu, 1 Sep 2022 15:30:17 +0200 Subject: [PATCH 14/17] Break long words in CodeMirror (#2044) * Break long words in CodeMirror Credit to Marko Letic: https://stackoverflow.com/a/57377527 * Move rule to more general CodeMirror component --- ts/editor/CodeMirror.svelte | 9 +++++++-- ts/editor/plain-text-input/PlainTextInput.svelte | 1 - 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/ts/editor/CodeMirror.svelte b/ts/editor/CodeMirror.svelte index f9eaeeb41..12d3e73b2 100644 --- a/ts/editor/CodeMirror.svelte +++ b/ts/editor/CodeMirror.svelte @@ -88,7 +88,12 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
diff --git a/ts/editor/plain-text-input/PlainTextInput.svelte b/ts/editor/plain-text-input/PlainTextInput.svelte index d3479f418..96792d56a 100644 --- a/ts/editor/plain-text-input/PlainTextInput.svelte +++ b/ts/editor/plain-text-input/PlainTextInput.svelte @@ -174,7 +174,6 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html border: none; border-radius: 5px; } - :global(.CodeMirror) { background: var(--code-bg); } From d9e2d7d3eb08aaf88cecff51279cfa5df94c0c20 Mon Sep 17 00:00:00 2001 From: Matthias Metelka <62722460+kleinerpirat@users.noreply.github.com> Date: Thu, 1 Sep 2022 15:31:47 +0200 Subject: [PATCH 15/17] Break long words in pre tags (#2045) * Break long words in pre tags * Move rule to editable-base.scss --- ts/editable/editable-base.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ts/editable/editable-base.scss b/ts/editable/editable-base.scss index ac31a51ea..a0df4e886 100644 --- a/ts/editable/editable-base.scss +++ b/ts/editable/editable-base.scss @@ -21,3 +21,7 @@ p { :host(.night-mode) { @include scrollbar.night-mode; } + +pre { + white-space: pre-wrap; +} From 31b7464c67343905e5a46001835b9f18c98af90b Mon Sep 17 00:00:00 2001 From: RumovZ Date: Fri, 2 Sep 2022 03:22:49 +0200 Subject: [PATCH 16/17] Add card meta for persisting custom scheduling state (#2040) * Add card meta for persisting custom scheduling state * Rename meta -> custom_data * Enforce limits on size of custom data Large values will slow down table scans of the cards table, and it's easier to be strict now and possibly relax things in the future than the opposite. * Pack card states and customData into a single message + default customData to empty if it can't be parsed Co-authored-by: Damien Elmes --- proto/anki/cards.proto | 1 + proto/anki/scheduler.proto | 7 +++ pylib/anki/cards.py | 6 +-- pylib/anki/scheduler/v3.py | 13 ++++- qt/aqt/mediasrv.py | 18 +++---- qt/aqt/reviewer.py | 23 ++++++--- rslib/src/backend/card.rs | 5 ++ rslib/src/backend/scheduler/answering.rs | 1 + rslib/src/card/mod.rs | 3 ++ rslib/src/scheduler/answering/mod.rs | 4 ++ rslib/src/scheduler/answering/preview.rs | 4 ++ rslib/src/storage/card/data.rs | 62 +++++++++++++++++++++--- rslib/src/storage/card/mod.rs | 1 + rslib/src/sync/mod.rs | 9 +++- ts/reviewer/answering.ts | 35 ++++++++----- 15 files changed, 151 insertions(+), 41 deletions(-) diff --git a/proto/anki/cards.proto b/proto/anki/cards.proto index d33767238..afb196b38 100644 --- a/proto/anki/cards.proto +++ b/proto/anki/cards.proto @@ -44,6 +44,7 @@ message Card { int64 original_deck_id = 16; uint32 flags = 17; optional uint32 original_position = 18; + string custom_data = 19; } message UpdateCardsRequest { diff --git a/proto/anki/scheduler.proto b/proto/anki/scheduler.proto index 6fd2cda57..a68402271 100644 --- a/proto/anki/scheduler.proto +++ b/proto/anki/scheduler.proto @@ -240,6 +240,7 @@ message CardAnswer { Rating rating = 4; int64 answered_at_millis = 5; uint32 milliseconds_taken = 6; + string custom_data = 7; } message CustomStudyRequest { @@ -303,3 +304,9 @@ message RepositionDefaultsResponse { bool random = 1; bool shift = 2; } + +// Data required to support the v3 scheduler's custom scheduling feature +message CustomScheduling { + NextCardStates states = 1; + string custom_data = 2; +} diff --git a/pylib/anki/cards.py b/pylib/anki/cards.py index 607792514..56b83e788 100644 --- a/pylib/anki/cards.py +++ b/pylib/anki/cards.py @@ -92,6 +92,7 @@ class Card(DeprecatedNamesMixin): self.original_position = ( card.original_position if card.HasField("original_position") else None ) + self.custom_data = card.custom_data def _to_backend_card(self) -> cards_pb2.Card: # mtime & usn are set by backend @@ -111,9 +112,8 @@ class Card(DeprecatedNamesMixin): original_due=self.odue, original_deck_id=self.odid, flags=self.flags, - original_position=self.original_position - if self.original_position is not None - else None, + original_position=self.original_position, + custom_data=self.custom_data, ) def flush(self) -> None: diff --git a/pylib/anki/scheduler/v3.py b/pylib/anki/scheduler/v3.py index c23d26922..faf5c1ccf 100644 --- a/pylib/anki/scheduler/v3.py +++ b/pylib/anki/scheduler/v3.py @@ -31,6 +31,7 @@ QueuedCards = scheduler_pb2.QueuedCards SchedulingState = scheduler_pb2.SchedulingState NextStates = scheduler_pb2.NextCardStates CardAnswer = scheduler_pb2.CardAnswer +CustomScheduling = scheduler_pb2.CustomScheduling class Scheduler(SchedulerBaseWithLegacy): @@ -61,7 +62,12 @@ class Scheduler(SchedulerBaseWithLegacy): ########################################################################## def build_answer( - self, *, card: Card, states: NextStates, rating: CardAnswer.Rating.V + self, + *, + card: Card, + states: NextStates, + custom_data: str, + rating: CardAnswer.Rating.V, ) -> CardAnswer: "Build input for answer_card()." if rating == CardAnswer.AGAIN: @@ -79,6 +85,7 @@ class Scheduler(SchedulerBaseWithLegacy): card_id=card.id, current_state=states.current, new_state=new_state, + custom_data=custom_data, rating=rating, answered_at_millis=int_time(1000), milliseconds_taken=card.time_taken(capped=False), @@ -163,7 +170,9 @@ class Scheduler(SchedulerBaseWithLegacy): states = self.col._backend.get_next_card_states(card.id) changes = self.answer_card( - self.build_answer(card=card, states=states, rating=rating) + self.build_answer( + card=card, states=states, custom_data=card.custom_data, rating=rating + ) ) # tests assume card will be mutated, so we need to reload it diff --git a/qt/aqt/mediasrv.py b/qt/aqt/mediasrv.py index dba07a44a..f28236dd1 100644 --- a/qt/aqt/mediasrv.py +++ b/qt/aqt/mediasrv.py @@ -27,7 +27,7 @@ from anki import hooks from anki._vendor import stringcase from anki.collection import OpChanges from anki.decks import DeckConfigsForUpdate, UpdateDeckConfigs -from anki.scheduler.v3 import NextStates +from anki.scheduler.v3 import CustomScheduling from anki.utils import dev_mode from aqt.changenotetype import ChangeNotetypeDialog from aqt.deckoptions import DeckOptionsDialog @@ -412,18 +412,18 @@ def update_deck_configs() -> bytes: return b"" -def next_card_states() -> bytes: - if states := aqt.mw.reviewer.get_next_states(): - return states.SerializeToString() +def get_custom_scheduling() -> bytes: + if scheduling := aqt.mw.reviewer.get_custom_scheduling(): + return scheduling.SerializeToString() else: return b"" -def set_next_card_states() -> bytes: +def set_custom_scheduling() -> bytes: key = request.headers.get("key", "") - input = NextStates() + input = CustomScheduling() input.ParseFromString(request.data) - aqt.mw.reviewer.set_next_states(key, input) + aqt.mw.reviewer.set_custom_scheduling(key, input) return b"" @@ -455,8 +455,8 @@ post_handler_list = [ congrats_info, get_deck_configs_for_update, update_deck_configs, - next_card_states, - set_next_card_states, + get_custom_scheduling, + set_custom_scheduling, change_notetype, import_csv, ] diff --git a/qt/aqt/reviewer.py b/qt/aqt/reviewer.py index ca984121a..09b3e32cb 100644 --- a/qt/aqt/reviewer.py +++ b/qt/aqt/reviewer.py @@ -17,7 +17,7 @@ from anki import hooks from anki.cards import Card, CardId from anki.collection import Config, OpChanges, OpChangesWithCount from anki.scheduler.base import ScheduleCardsAsNew -from anki.scheduler.v3 import CardAnswer, NextStates, QueuedCards +from anki.scheduler.v3 import CardAnswer, CustomScheduling, NextStates, QueuedCards from anki.scheduler.v3 import Scheduler as V3Scheduler from anki.tags import MARKED_TAG from anki.types import assert_exhaustive @@ -82,11 +82,14 @@ class V3CardInfo: queued_cards: QueuedCards next_states: NextStates + custom_data: str @staticmethod def from_queue(queued_cards: QueuedCards) -> V3CardInfo: return V3CardInfo( - queued_cards=queued_cards, next_states=queued_cards.cards[0].next_states + queued_cards=queued_cards, + next_states=queued_cards.cards[0].next_states, + custom_data=queued_cards.cards[0].card.custom_data, ) def top_card(self) -> QueuedCards.QueuedCard: @@ -259,23 +262,24 @@ class Reviewer: self.card = Card(self.mw.col, backend_card=self._v3.top_card().card) self.card.start_timer() - def get_next_states(self) -> NextStates | None: + def get_custom_scheduling(self) -> CustomScheduling | None: if v3 := self._v3: - return v3.next_states + return CustomScheduling(states=v3.next_states, custom_data=v3.custom_data) else: return None - def set_next_states(self, key: str, states: NextStates) -> None: + def set_custom_scheduling(self, key: str, scheduling: CustomScheduling) -> None: if key != self._state_mutation_key: return if v3 := self._v3: - v3.next_states = states + v3.next_states = scheduling.states + v3.custom_data = scheduling.custom_data def _run_state_mutation_hook(self) -> None: if self._v3 and (js := self._state_mutation_js): self.web.eval( - f"anki.mutateNextCardStates('{self._state_mutation_key}', (states) => {{ {js} }})" + f"anki.mutateNextCardStates('{self._state_mutation_key}', (states, customData) => {{ {js} }})" ) # Audio @@ -431,7 +435,10 @@ class Reviewer: if (v3 := self._v3) and (sched := cast(V3Scheduler, self.mw.col.sched)): answer = sched.build_answer( - card=self.card, states=v3.next_states, rating=v3.rating_from_ease(ease) + card=self.card, + states=v3.next_states, + custom_data=v3.custom_data, + rating=v3.rating_from_ease(ease), ) def after_answer(changes: OpChanges) -> None: diff --git a/rslib/src/backend/card.rs b/rslib/src/backend/card.rs index fad64614d..0cafc0f8c 100644 --- a/rslib/src/backend/card.rs +++ b/rslib/src/backend/card.rs @@ -26,6 +26,9 @@ impl CardsService for Backend { .into_iter() .map(TryInto::try_into) .collect::, AnkiError>>()?; + for card in &cards { + card.validate_custom_data()?; + } col.update_cards_maybe_undoable(cards, !input.skip_undo_entry) }) .map(Into::into) @@ -87,6 +90,7 @@ impl TryFrom for Card { original_deck_id: DeckId(c.original_deck_id), flags: c.flags as u8, original_position: c.original_position, + custom_data: c.custom_data, }) } } @@ -112,6 +116,7 @@ impl From for pb::Card { original_deck_id: c.original_deck_id.0, flags: c.flags as u32, original_position: c.original_position.map(Into::into), + custom_data: c.custom_data, } } } diff --git a/rslib/src/backend/scheduler/answering.rs b/rslib/src/backend/scheduler/answering.rs index 3d3447408..7e69010cc 100644 --- a/rslib/src/backend/scheduler/answering.rs +++ b/rslib/src/backend/scheduler/answering.rs @@ -19,6 +19,7 @@ impl From for CardAnswer { new_state: answer.new_state.unwrap_or_default().into(), answered_at: TimestampMillis(answer.answered_at_millis), milliseconds_taken: answer.milliseconds_taken, + custom_data: answer.custom_data, } } } diff --git a/rslib/src/card/mod.rs b/rslib/src/card/mod.rs index 41c11ec0d..6fd1a4bda 100644 --- a/rslib/src/card/mod.rs +++ b/rslib/src/card/mod.rs @@ -79,6 +79,8 @@ pub struct Card { pub(crate) flags: u8, /// The position in the new queue before leaving it. pub(crate) original_position: Option, + /// JSON object or empty; exposed through the reviewer for persisting custom state + pub(crate) custom_data: String, } impl Default for Card { @@ -102,6 +104,7 @@ impl Default for Card { original_deck_id: DeckId(0), flags: 0, original_position: None, + custom_data: String::new(), } } } diff --git a/rslib/src/scheduler/answering/mod.rs b/rslib/src/scheduler/answering/mod.rs index 861fcba1d..003f64dbc 100644 --- a/rslib/src/scheduler/answering/mod.rs +++ b/rslib/src/scheduler/answering/mod.rs @@ -41,6 +41,7 @@ pub struct CardAnswer { pub rating: Rating, pub answered_at: TimestampMillis, pub milliseconds_taken: u32, + pub custom_data: String, } impl CardAnswer { @@ -273,6 +274,8 @@ impl Collection { self.maybe_bury_siblings(&original, &updater.config)?; let timing = updater.timing; let mut card = updater.into_card(); + card.custom_data = answer.custom_data.clone(); + card.validate_custom_data()?; self.update_card_inner(&mut card, original, usn)?; if answer.new_state.leeched() { self.add_leech_tag(card.note_id)?; @@ -419,6 +422,7 @@ pub mod test_helpers { rating, answered_at: TimestampMillis::now(), milliseconds_taken: 0, + custom_data: String::new(), })?; Ok(PostAnswerState { card_id: queued.card.id, diff --git a/rslib/src/scheduler/answering/preview.rs b/rslib/src/scheduler/answering/preview.rs index 1ef3b6cf5..70c50850b 100644 --- a/rslib/src/scheduler/answering/preview.rs +++ b/rslib/src/scheduler/answering/preview.rs @@ -92,6 +92,7 @@ mod test { rating: Rating::Again, answered_at: TimestampMillis::now(), milliseconds_taken: 0, + custom_data: String::new(), })?; c = col.storage.get_card(c.id)?.unwrap(); @@ -106,6 +107,7 @@ mod test { rating: Rating::Hard, answered_at: TimestampMillis::now(), milliseconds_taken: 0, + custom_data: String::new(), })?; c = col.storage.get_card(c.id)?.unwrap(); assert_eq!(c.queue, CardQueue::PreviewRepeat); @@ -119,6 +121,7 @@ mod test { rating: Rating::Good, answered_at: TimestampMillis::now(), milliseconds_taken: 0, + custom_data: String::new(), })?; c = col.storage.get_card(c.id)?.unwrap(); assert_eq!(c.queue, CardQueue::PreviewRepeat); @@ -132,6 +135,7 @@ mod test { rating: Rating::Easy, answered_at: TimestampMillis::now(), milliseconds_taken: 0, + custom_data: String::new(), })?; c = col.storage.get_card(c.id)?.unwrap(); assert_eq!(c.queue, CardQueue::DayLearn); diff --git a/rslib/src/storage/card/data.rs b/rslib/src/storage/card/data.rs index 5f7338497..4c303c70a 100644 --- a/rslib/src/storage/card/data.rs +++ b/rslib/src/storage/card/data.rs @@ -1,32 +1,45 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html +use std::collections::HashMap; + use rusqlite::{ types::{FromSql, FromSqlError, ToSqlOutput, ValueRef}, ToSql, }; use serde_derive::{Deserialize, Serialize}; +use serde_json::Value; use crate::{prelude::*, serde::default_on_invalid}; /// Helper for serdeing the card data column. #[derive(Debug, Clone, PartialEq, Default, Serialize, Deserialize)] #[serde(default)] -pub(super) struct CardData { +pub(crate) struct CardData { #[serde( skip_serializing_if = "Option::is_none", rename = "pos", deserialize_with = "default_on_invalid" )] pub(crate) original_position: Option, + /// A string representation of a JSON object storing optional data + /// associated with the card, so v3 custom scheduling code can persist + /// state. + #[serde(default, rename = "cd", skip_serializing_if = "meta_is_empty")] + pub(crate) custom_data: String, } impl CardData { - pub(super) fn from_card(card: &Card) -> Self { + pub(crate) fn from_card(card: &Card) -> Self { Self { original_position: card.original_position, + custom_data: card.custom_data.clone(), } } + + pub(crate) fn from_str(s: &str) -> Self { + serde_json::from_str(s).unwrap_or_default() + } } impl FromSql for CardData { @@ -53,8 +66,45 @@ pub(crate) fn card_data_string(card: &Card) -> String { serde_json::to_string(&CardData::from_card(card)).unwrap() } -/// Extract original position from JSON `data`. -pub(crate) fn original_position_from_card_data(card_data: &str) -> Option { - let data: CardData = serde_json::from_str(card_data).unwrap_or_default(); - data.original_position +fn meta_is_empty(s: &str) -> bool { + matches!(s, "" | "{}") +} + +fn validate_custom_data(json_str: &str) -> Result<()> { + if !meta_is_empty(json_str) { + let object: HashMap<&str, Value> = serde_json::from_str(json_str) + .map_err(|e| AnkiError::invalid_input(format!("custom data not an object: {e}")))?; + if object.keys().any(|k| k.as_bytes().len() > 8) { + return Err(AnkiError::invalid_input( + "custom data keys must be <= 8 bytes", + )); + } + if json_str.len() > 100 { + return Err(AnkiError::invalid_input( + "serialized custom data must be under 100 bytes", + )); + } + } + Ok(()) +} + +impl Card { + pub(crate) fn validate_custom_data(&self) -> Result<()> { + validate_custom_data(&self.custom_data) + } +} + +#[cfg(test)] +mod test { + use super::*; + #[test] + fn validation() { + assert!(validate_custom_data("").is_ok()); + assert!(validate_custom_data("{}").is_ok()); + assert!(validate_custom_data(r#"{"foo": 5}"#).is_ok()); + assert!(validate_custom_data(r#"["foo"]"#).is_err()); + assert!(validate_custom_data(r#"{"日": 5}"#).is_ok()); + assert!(validate_custom_data(r#"{"日本語": 5}"#).is_err()); + assert!(validate_custom_data(&format!(r#"{{"foo": "{}"}}"#, "x".repeat(100))).is_err()); + } } diff --git a/rslib/src/storage/card/mod.rs b/rslib/src/storage/card/mod.rs index 2e0549d06..adc81731b 100644 --- a/rslib/src/storage/card/mod.rs +++ b/rslib/src/storage/card/mod.rs @@ -69,6 +69,7 @@ fn row_to_card(row: &Row) -> result::Result { original_deck_id: row.get(15)?, flags: row.get(16)?, original_position: data.original_position, + custom_data: data.custom_data, }) } diff --git a/rslib/src/sync/mod.rs b/rslib/src/sync/mod.rs index eb3c43d6c..3f60686a4 100644 --- a/rslib/src/sync/mod.rs +++ b/rslib/src/sync/mod.rs @@ -28,7 +28,7 @@ use crate::{ revlog::RevlogEntry, serde::{default_on_invalid, deserialize_int_from_number}, storage::{ - card::data::{card_data_string, original_position_from_card_data}, + card::data::{card_data_string, CardData}, open_and_check_sqlite_file, SchemaVersion, }, tags::{join_tags, split_tags, Tag}, @@ -1081,6 +1081,10 @@ impl Collection { impl From for Card { fn from(e: CardEntry) -> Self { + let CardData { + original_position, + custom_data, + } = CardData::from_str(&e.data); Card { id: e.id, note_id: e.nid, @@ -1099,7 +1103,8 @@ impl From for Card { original_due: e.odue, original_deck_id: e.odid, flags: e.flags, - original_position: original_position_from_card_data(&e.data), + original_position, + custom_data, } } } diff --git a/ts/reviewer/answering.ts b/ts/reviewer/answering.ts index 5aea76d4e..f3a633763 100644 --- a/ts/reviewer/answering.ts +++ b/ts/reviewer/answering.ts @@ -4,25 +4,38 @@ import { postRequest } from "../lib/postrequest"; import { Scheduler } from "../lib/proto"; -async function getNextStates(): Promise { - return Scheduler.NextCardStates.decode( - await postRequest("/_anki/nextCardStates", ""), +async function getCustomScheduling(): Promise { + return Scheduler.CustomScheduling.decode( + await postRequest("/_anki/getCustomScheduling", ""), ); } -async function setNextStates( +async function setCustomScheduling( key: string, - states: Scheduler.NextCardStates, + scheduling: Scheduler.CustomScheduling, ): Promise { - const data: Uint8Array = Scheduler.NextCardStates.encode(states).finish(); - await postRequest("/_anki/setNextCardStates", data, { key }); + const bytes = Scheduler.CustomScheduling.encode(scheduling).finish(); + await postRequest("/_anki/setCustomScheduling", bytes, { key }); } export async function mutateNextCardStates( key: string, - mutator: (states: Scheduler.NextCardStates) => void, + mutator: ( + states: Scheduler.NextCardStates, + customData: Record, + ) => void, ): Promise { - const states = await getNextStates(); - mutator(states); - await setNextStates(key, states); + const scheduling = await getCustomScheduling(); + let customData = {}; + try { + customData = JSON.parse(scheduling.customData); + } catch { + // can't be parsed + } + + mutator(scheduling.states!, customData); + + scheduling.customData = JSON.stringify(customData); + + await setCustomScheduling(key, scheduling); } From 8cf829d442f4e899dc95738aee168a707b1c2bb2 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Fri, 2 Sep 2022 15:04:10 +1000 Subject: [PATCH 17/17] Update translations --- repos.bzl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/repos.bzl b/repos.bzl index 4f05a1649..b5f1aa798 100644 --- a/repos.bzl +++ b/repos.bzl @@ -115,12 +115,12 @@ def register_repos(): ################ core_i18n_repo = "anki-core-i18n" - core_i18n_commit = "86d2edc647c030827a69790225efde23e720bccf" - core_i18n_zip_csum = "d9f51d4baea7ee79a05e35852eefece29646397f8e89608382794b00eac8aab7" + core_i18n_commit = "d8673a9e101ca90381d5822bd51766239ee52cc9" + core_i18n_zip_csum = "54b33e668d867bfc26b541cc2ca014441ae82e1e12865028fa6ad19192d52453" qtftl_i18n_repo = "anki-desktop-ftl" - qtftl_i18n_commit = "fbe70aa7b3a0ce8ffb7d34f7392e00f71af541bf" - qtftl_i18n_zip_csum = "a1a01f6822ee048fa7c570a27410c44c8a5cca76cba97d4bbff84ed6cfbf75a6" + qtftl_i18n_commit = "77881685cf615888c8ce0fe8aed6f3ae665bcfc5" + qtftl_i18n_zip_csum = "c5d48ea05038009d390351c63ecd7b67fe7177b8a66db6087c3b8a8bcc28a85c" i18n_build_content = """ filegroup(