From 1dea8babee918990d51a2479be40715ebd83b75c Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sun, 7 Mar 2021 23:43:18 +1000 Subject: [PATCH] use native boolkey instead of separate getters/setters Makes it easier to add new config settings in the future, especially if we don't need to export them via protobuf. --- rslib/src/backend/config.rs | 22 +++++ rslib/src/backend/mod.rs | 5 +- rslib/src/config.rs | 150 ++++++++++------------------------ rslib/src/dbcheck.rs | 2 +- rslib/src/decks/tree.rs | 2 +- rslib/src/findreplace.rs | 3 +- rslib/src/notes/mod.rs | 15 ++-- rslib/src/notetype/mod.rs | 3 +- rslib/src/preferences.rs | 19 +++-- rslib/src/prelude.rs | 1 + rslib/src/search/cards.rs | 8 +- rslib/src/search/sqlwriter.rs | 3 +- rslib/src/stats/graphs.rs | 17 ++-- 13 files changed, 116 insertions(+), 134 deletions(-) create mode 100644 rslib/src/backend/config.rs diff --git a/rslib/src/backend/config.rs b/rslib/src/backend/config.rs new file mode 100644 index 000000000..c9fe7665b --- /dev/null +++ b/rslib/src/backend/config.rs @@ -0,0 +1,22 @@ +// Copyright: Ankitects Pty Ltd and contributors +// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html + +use crate::{backend_proto as pb, config::BoolKey}; +use pb::config::bool::Key; + +impl From for BoolKey { + fn from(k: Key) -> Self { + match k { + Key::BrowserSortBackwards => BoolKey::BrowserSortBackwards, + Key::PreviewBothSides => BoolKey::PreviewBothSides, + Key::CollapseTags => BoolKey::CollapseTags, + Key::CollapseNotetypes => BoolKey::CollapseNotetypes, + Key::CollapseDecks => BoolKey::CollapseDecks, + Key::CollapseSavedSearches => BoolKey::CollapseSavedSearches, + Key::CollapseToday => BoolKey::CollapseToday, + Key::CollapseCardState => BoolKey::CollapseCardState, + Key::CollapseFlags => BoolKey::CollapseFlags, + Key::Sched2021 => BoolKey::Sched2021, + } + } +} diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index 427182b51..02133ad31 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -2,6 +2,7 @@ // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html mod card; +mod config; mod dbproxy; mod generic; mod http_sync_server; @@ -1513,13 +1514,13 @@ impl BackendService for Backend { fn get_config_bool(&self, input: pb::config::Bool) -> BackendResult { self.with_col(|col| { Ok(pb::Bool { - val: col.get_bool(input), + val: col.get_bool(input.key().into()), }) }) } fn set_config_bool(&self, input: pb::SetConfigBoolIn) -> BackendResult { - self.with_col(|col| col.transact(None, |col| col.set_bool(input))) + self.with_col(|col| col.transact(None, |col| col.set_bool(input.key().into(), input.value))) .map(Into::into) } diff --git a/rslib/src/config.rs b/rslib/src/config.rs index 5e26ddd40..35712c0e3 100644 --- a/rslib/src/config.rs +++ b/rslib/src/config.rs @@ -5,7 +5,6 @@ use crate::{ backend_proto as pb, collection::Collection, decks::DeckID, err::Result, notetype::NoteTypeID, timestamp::TimestampSecs, }; -pub use pb::config::bool::Key as BoolKey; pub use pb::config::string::Key as StringKey; use serde::{de::DeserializeOwned, Serialize}; use serde_aux::field_attributes::deserialize_bool_from_anything; @@ -42,21 +41,10 @@ pub(crate) fn schema11_config_as_string() -> String { #[derive(IntoStaticStr)] #[strum(serialize_all = "camelCase")] pub(crate) enum ConfigKey { - CardCountsSeparateInactive, - CollapseCardState, - CollapseDecks, - CollapseFlags, - CollapseNotetypes, - CollapseSavedSearches, - CollapseTags, - CollapseToday, CreationOffset, FirstDayOfWeek, - FutureDueShowBacklog, LocalOffset, - PreviewBothSides, Rollover, - Sched2021, SetDueBrowser, SetDueReviewer, @@ -64,8 +52,6 @@ pub(crate) enum ConfigKey { AnswerTimeLimitSecs, #[strum(to_string = "sortType")] BrowserSortKind, - #[strum(to_string = "sortBackwards")] - BrowserSortReverse, #[strum(to_string = "curDeck")] CurrentDeckID, #[strum(to_string = "curModel")] @@ -78,10 +64,29 @@ pub(crate) enum ConfigKey { NewReviewMix, #[strum(to_string = "nextPos")] NextNewCardPosition, - #[strum(to_string = "normalize_note_text")] - NormalizeNoteText, #[strum(to_string = "schedVer")] SchedulerVersion, +} + +#[derive(Debug, Clone, Copy, IntoStaticStr)] +#[strum(serialize_all = "camelCase")] +pub enum BoolKey { + CardCountsSeparateInactive, + CollapseCardState, + CollapseDecks, + CollapseFlags, + CollapseNotetypes, + CollapseSavedSearches, + CollapseTags, + CollapseToday, + FutureDueShowBacklog, + PreviewBothSides, + Sched2021, + + #[strum(to_string = "sortBackwards")] + BrowserSortBackwards, + #[strum(to_string = "normalize_note_text")] + NormalizeNoteText, #[strum(to_string = "dayLearnFirst")] ShowDayLearningCardsFirst, #[strum(to_string = "estTimes")] @@ -90,23 +95,6 @@ pub(crate) enum ConfigKey { ShowRemainingDueCountsInStudy, } -impl From for ConfigKey { - fn from(key: BoolKey) -> Self { - match key { - BoolKey::BrowserSortBackwards => ConfigKey::BrowserSortReverse, - BoolKey::CollapseCardState => ConfigKey::CollapseCardState, - BoolKey::CollapseDecks => ConfigKey::CollapseDecks, - BoolKey::CollapseFlags => ConfigKey::CollapseFlags, - BoolKey::CollapseNotetypes => ConfigKey::CollapseNotetypes, - BoolKey::CollapseSavedSearches => ConfigKey::CollapseSavedSearches, - BoolKey::CollapseTags => ConfigKey::CollapseTags, - BoolKey::CollapseToday => ConfigKey::CollapseToday, - BoolKey::PreviewBothSides => ConfigKey::PreviewBothSides, - BoolKey::Sched2021 => ConfigKey::Sched2021, - } - } -} - impl From for ConfigKey { fn from(key: StringKey) -> Self { match key { @@ -169,13 +157,31 @@ impl Collection { self.storage.remove_config(key.into()) } - pub(crate) fn get_browser_sort_kind(&self) -> SortKind { - self.get_config_default(ConfigKey::BrowserSortKind) + pub(crate) fn get_bool(&self, key: BoolKey) -> bool { + match key { + BoolKey::BrowserSortBackwards => { + // older clients were storing this as an int + self.get_config_default::(BoolKey::BrowserSortBackwards) + .0 + } + + // some keys default to true + BoolKey::FutureDueShowBacklog + | BoolKey::ShowRemainingDueCountsInStudy + | BoolKey::CardCountsSeparateInactive + | BoolKey::NormalizeNoteText => self.get_config_optional(key).unwrap_or(true), + + // other options default to false + other => self.get_config_default(other), + } } - pub(crate) fn get_browser_sort_reverse(&self) -> bool { - let b: BoolLike = self.get_config_default(ConfigKey::BrowserSortReverse); - b.0 + pub(crate) fn set_bool(&self, key: BoolKey, value: bool) -> Result<()> { + self.set_config(key, &value) + } + + pub(crate) fn get_browser_sort_kind(&self) -> SortKind { + self.get_config_default(ConfigKey::BrowserSortKind) } pub(crate) fn get_current_deck_id(&self) -> DeckID { @@ -256,12 +262,6 @@ impl Collection { self.set_config(ConfigKey::LearnAheadSecs, &secs) } - /// This is a stop-gap solution until we can decouple searching from canonical storage. - pub(crate) fn normalize_note_text(&self) -> bool { - self.get_config_optional(ConfigKey::NormalizeNoteText) - .unwrap_or(true) - } - pub(crate) fn get_new_review_mix(&self) -> NewReviewMix { match self.get_config_default::(ConfigKey::NewReviewMix) { 1 => NewReviewMix::ReviewsFirst, @@ -283,42 +283,6 @@ impl Collection { self.set_config(ConfigKey::FirstDayOfWeek, &weekday) } - pub(crate) fn get_card_counts_separate_inactive(&self) -> bool { - self.get_config_optional(ConfigKey::CardCountsSeparateInactive) - .unwrap_or(true) - } - - pub(crate) fn set_card_counts_separate_inactive(&self, separate: bool) -> Result<()> { - self.set_config(ConfigKey::CardCountsSeparateInactive, &separate) - } - - pub(crate) fn get_future_due_show_backlog(&self) -> bool { - self.get_config_optional(ConfigKey::FutureDueShowBacklog) - .unwrap_or(true) - } - - pub(crate) fn set_future_due_show_backlog(&self, show: bool) -> Result<()> { - self.set_config(ConfigKey::FutureDueShowBacklog, &show) - } - - pub(crate) fn get_show_due_counts(&self) -> bool { - self.get_config_optional(ConfigKey::ShowRemainingDueCountsInStudy) - .unwrap_or(true) - } - - pub(crate) fn set_show_due_counts(&self, on: bool) -> Result<()> { - self.set_config(ConfigKey::ShowRemainingDueCountsInStudy, &on) - } - - pub(crate) fn get_show_intervals_above_buttons(&self) -> bool { - self.get_config_optional(ConfigKey::ShowIntervalsAboveAnswerButtons) - .unwrap_or(true) - } - - pub(crate) fn set_show_intervals_above_buttons(&self, on: bool) -> Result<()> { - self.set_config(ConfigKey::ShowIntervalsAboveAnswerButtons, &on) - } - pub(crate) fn get_answer_time_limit_secs(&self) -> u32 { self.get_config_optional(ConfigKey::AnswerTimeLimitSecs) .unwrap_or_default() @@ -328,15 +292,6 @@ impl Collection { self.set_config(ConfigKey::AnswerTimeLimitSecs, &secs) } - pub(crate) fn get_day_learn_first(&self) -> bool { - self.get_config_optional(ConfigKey::ShowDayLearningCardsFirst) - .unwrap_or_default() - } - - pub(crate) fn set_day_learn_first(&self, on: bool) -> Result<()> { - self.set_config(ConfigKey::ShowDayLearningCardsFirst, &on) - } - pub(crate) fn get_last_unburied_day(&self) -> u32 { self.get_config_optional(ConfigKey::LastUnburiedDay) .unwrap_or_default() @@ -346,23 +301,6 @@ impl Collection { self.set_config(ConfigKey::LastUnburiedDay, &day) } - #[allow(clippy::match_single_binding)] - pub(crate) fn get_bool(&self, config: pb::config::Bool) -> bool { - self.get_bool_key(config.key()) - } - - #[allow(clippy::match_single_binding)] - pub(crate) fn get_bool_key(&self, key: BoolKey) -> bool { - match key { - // all options default to false at the moment - other => self.get_config_default(ConfigKey::from(other)), - } - } - - pub(crate) fn set_bool(&self, input: pb::SetConfigBoolIn) -> Result<()> { - self.set_config(ConfigKey::from(input.key()), &input.value) - } - pub(crate) fn get_string(&self, config: pb::config::String) -> String { let key = config.key(); let default = match key { diff --git a/rslib/src/dbcheck.rs b/rslib/src/dbcheck.rs index caa00a96c..e6c5e50d6 100644 --- a/rslib/src/dbcheck.rs +++ b/rslib/src/dbcheck.rs @@ -230,7 +230,7 @@ impl Collection { F: FnMut(DatabaseCheckProgress, bool), { let nids_by_notetype = self.storage.all_note_ids_by_notetype()?; - let norm = self.normalize_note_text(); + let norm = self.get_bool(BoolKey::NormalizeNoteText); let usn = self.usn()?; let stamp = TimestampMillis::now(); diff --git a/rslib/src/decks/tree.rs b/rslib/src/decks/tree.rs index 0c5088c50..683e18f9b 100644 --- a/rslib/src/decks/tree.rs +++ b/rslib/src/decks/tree.rs @@ -283,7 +283,7 @@ impl Collection { let dconf = self.storage.get_deck_config_map()?; add_counts(&mut tree, &counts); if self.scheduler_version() == SchedulerVersion::V2 - && !self.get_bool_key(BoolKey::Sched2021) + && !self.get_bool(BoolKey::Sched2021) { apply_limits_v2_old( &mut tree, diff --git a/rslib/src/findreplace.rs b/rslib/src/findreplace.rs index 516acb0db..8d97eb1ec 100644 --- a/rslib/src/findreplace.rs +++ b/rslib/src/findreplace.rs @@ -5,6 +5,7 @@ use crate::{ collection::Collection, err::{AnkiError, Result}, notes::{NoteID, TransformNoteOutput}, + prelude::*, text::normalize_to_nfc, }; use regex::Regex; @@ -46,7 +47,7 @@ impl Collection { field_name: Option, ) -> Result { self.transact(None, |col| { - let norm = col.normalize_note_text(); + let norm = col.get_bool(BoolKey::NormalizeNoteText); let search = if norm { normalize_to_nfc(search_re) } else { diff --git a/rslib/src/notes/mod.rs b/rslib/src/notes/mod.rs index b824b7eb7..c8d239738 100644 --- a/rslib/src/notes/mod.rs +++ b/rslib/src/notes/mod.rs @@ -305,7 +305,7 @@ impl Collection { .get_notetype(note.notetype_id)? .ok_or_else(|| AnkiError::invalid_input("missing note type"))?; let ctx = CardGenContext::new(&nt, col.usn()?); - let norm = col.normalize_note_text(); + let norm = col.get_bool(BoolKey::NormalizeNoteText); col.add_note_inner(&ctx, note, did, norm) }) } @@ -345,7 +345,7 @@ impl Collection { .get_notetype(note.notetype_id)? .ok_or_else(|| AnkiError::invalid_input("missing note type"))?; let ctx = CardGenContext::new(&nt, col.usn()?); - let norm = col.normalize_note_text(); + let norm = col.get_bool(BoolKey::NormalizeNoteText); col.update_note_inner_generating_cards(&ctx, note, &existing_note, true, norm) }) } @@ -431,7 +431,7 @@ impl Collection { F: FnMut(&mut Note, &NoteType) -> Result, { let nids_by_notetype = self.storage.note_ids_by_notetype(nids)?; - let norm = self.normalize_note_text(); + let norm = self.get_bool(BoolKey::NormalizeNoteText); let mut changed_notes = 0; let usn = self.usn()?; @@ -479,7 +479,7 @@ impl Collection { pub(crate) fn note_is_duplicate_or_empty(&self, note: &Note) -> Result { if let Some(field1) = note.fields.get(0) { - let field1 = if self.normalize_note_text() { + let field1 = if self.get_bool(BoolKey::NormalizeNoteText) { normalize_to_nfc(field1) } else { field1.into() @@ -554,8 +554,8 @@ fn note_differs_from_db(existing_note: &mut Note, note: &mut Note) -> bool { mod test { use super::{anki_base91, field_checksum}; use crate::{ - collection::open_test_collection, config::ConfigKey, decks::DeckID, err::Result, - prelude::*, search::SortMode, + collection::open_test_collection, config::BoolKey, decks::DeckID, err::Result, prelude::*, + search::SortMode, }; #[test] @@ -642,8 +642,7 @@ mod test { // if normalization turned off, note text is entered as-is let mut note = nt.new_note(); note.fields[0] = "\u{fa47}".into(); - col.set_config(ConfigKey::NormalizeNoteText, &false) - .unwrap(); + col.set_config(BoolKey::NormalizeNoteText, &false).unwrap(); col.add_note(&mut note, DeckID(1))?; assert_eq!(note.fields[0], "\u{fa47}"); // normalized searches won't match diff --git a/rslib/src/notetype/mod.rs b/rslib/src/notetype/mod.rs index 6c19e94d8..8d03243a4 100644 --- a/rslib/src/notetype/mod.rs +++ b/rslib/src/notetype/mod.rs @@ -28,6 +28,7 @@ use crate::{ define_newtype, err::{AnkiError, Result}, notes::Note, + prelude::*, template::{FieldRequirements, ParsedTemplate}, text::ensure_string_in_nfc, timestamp::TimestampSecs, @@ -424,7 +425,7 @@ impl Collection { /// or fields have been added/removed/reordered. pub fn update_notetype(&mut self, nt: &mut NoteType, preserve_usn: bool) -> Result<()> { let existing = self.get_notetype(nt.id)?; - let norm = self.normalize_note_text(); + let norm = self.get_bool(BoolKey::NormalizeNoteText); nt.prepare_for_update(existing.as_ref().map(AsRef::as_ref))?; self.transact(None, |col| { if let Some(existing_notetype) = existing { diff --git a/rslib/src/preferences.rs b/rslib/src/preferences.rs index fcd82ffde..ae45fb9b9 100644 --- a/rslib/src/preferences.rs +++ b/rslib/src/preferences.rs @@ -7,6 +7,7 @@ use crate::{ Preferences, }, collection::Collection, + config::BoolKey, err::Result, scheduler::timing::local_minutes_west_for_stamp, }; @@ -39,11 +40,11 @@ impl Collection { crate::config::NewReviewMix::ReviewsFirst => NewRevMixPB::ReviewsFirst, crate::config::NewReviewMix::NewFirst => NewRevMixPB::NewFirst, } as i32, - show_remaining_due_counts: self.get_show_due_counts(), - show_intervals_on_buttons: self.get_show_intervals_above_buttons(), + show_remaining_due_counts: self.get_bool(BoolKey::ShowRemainingDueCountsInStudy), + show_intervals_on_buttons: self.get_bool(BoolKey::ShowIntervalsAboveAnswerButtons), time_limit_secs: self.get_answer_time_limit_secs(), new_timezone: self.get_creation_utc_offset().is_some(), - day_learn_first: self.get_day_learn_first(), + day_learn_first: self.get_bool(BoolKey::ShowDayLearningCardsFirst), }) } @@ -53,10 +54,16 @@ impl Collection { ) -> Result<()> { let s = settings; - self.set_day_learn_first(s.day_learn_first)?; + self.set_bool(BoolKey::ShowDayLearningCardsFirst, s.day_learn_first)?; + self.set_bool( + BoolKey::ShowRemainingDueCountsInStudy, + s.show_remaining_due_counts, + )?; + self.set_bool( + BoolKey::ShowIntervalsAboveAnswerButtons, + s.show_intervals_on_buttons, + )?; self.set_answer_time_limit_secs(s.time_limit_secs)?; - self.set_show_due_counts(s.show_remaining_due_counts)?; - self.set_show_intervals_above_buttons(s.show_intervals_on_buttons)?; self.set_learn_ahead_secs(s.learn_ahead_secs)?; self.set_new_review_mix(match s.new_review_mix() { diff --git a/rslib/src/prelude.rs b/rslib/src/prelude.rs index cc7669bd2..141173da2 100644 --- a/rslib/src/prelude.rs +++ b/rslib/src/prelude.rs @@ -4,6 +4,7 @@ pub use crate::{ card::{Card, CardID}, collection::Collection, + config::BoolKey, deckconf::{DeckConf, DeckConfID}, decks::{Deck, DeckID, DeckKind}, err::{AnkiError, Result}, diff --git a/rslib/src/search/cards.rs b/rslib/src/search/cards.rs index 4661f5f84..f4bc802eb 100644 --- a/rslib/src/search/cards.rs +++ b/rslib/src/search/cards.rs @@ -6,7 +6,11 @@ use super::{ sqlwriter::{RequiredTable, SqlWriter}, }; use crate::{ - card::CardID, card::CardType, collection::Collection, config::SortKind, err::Result, + card::CardID, + card::CardType, + collection::Collection, + config::{BoolKey, SortKind}, + err::Result, search::parser::parse, }; @@ -124,7 +128,7 @@ impl Collection { if mode == &SortMode::FromConfig { *mode = SortMode::Builtin { kind: self.get_browser_sort_kind(), - reverse: self.get_browser_sort_reverse(), + reverse: self.get_bool(BoolKey::BrowserSortBackwards), } } } diff --git a/rslib/src/search/sqlwriter.rs b/rslib/src/search/sqlwriter.rs index f65f93832..4c0c82dd1 100644 --- a/rslib/src/search/sqlwriter.rs +++ b/rslib/src/search/sqlwriter.rs @@ -9,6 +9,7 @@ use crate::{ err::Result, notes::field_checksum, notetype::NoteTypeID, + prelude::*, storage::ids_to_string, text::{ is_glob, matches_glob, normalize_to_nfc, strip_html_preserving_media_filenames, @@ -28,7 +29,7 @@ pub(crate) struct SqlWriter<'a> { impl SqlWriter<'_> { pub(crate) fn new(col: &mut Collection) -> SqlWriter<'_> { - let normalize_note_text = col.normalize_note_text(); + let normalize_note_text = col.get_bool(BoolKey::NormalizeNoteText); let sql = String::new(); let args = vec![]; SqlWriter { diff --git a/rslib/src/stats/graphs.rs b/rslib/src/stats/graphs.rs index 5d52c159b..084d3a4b2 100644 --- a/rslib/src/stats/graphs.rs +++ b/rslib/src/stats/graphs.rs @@ -2,7 +2,11 @@ // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html use crate::{ - backend_proto as pb, config::Weekday, prelude::*, revlog::RevlogEntry, search::SortMode, + backend_proto as pb, + config::{BoolKey, Weekday}, + prelude::*, + revlog::RevlogEntry, + search::SortMode, }; impl Collection { @@ -50,9 +54,9 @@ impl Collection { pub(crate) fn get_graph_preferences(&self) -> Result { Ok(pb::GraphPreferences { calendar_first_day_of_week: self.get_first_day_of_week() as i32, - card_counts_separate_inactive: self.get_card_counts_separate_inactive(), + card_counts_separate_inactive: self.get_bool(BoolKey::CardCountsSeparateInactive), browser_links_supported: true, - future_due_show_backlog: self.get_future_due_show_backlog(), + future_due_show_backlog: self.get_bool(BoolKey::FutureDueShowBacklog), }) } @@ -63,8 +67,11 @@ impl Collection { 6 => Weekday::Saturday, _ => Weekday::Sunday, })?; - self.set_card_counts_separate_inactive(prefs.card_counts_separate_inactive)?; - self.set_future_due_show_backlog(prefs.future_due_show_backlog)?; + self.set_bool( + BoolKey::CardCountsSeparateInactive, + prefs.card_counts_separate_inactive, + )?; + self.set_bool(BoolKey::FutureDueShowBacklog, prefs.future_due_show_backlog)?; Ok(()) } }