Remove from_config variant in pb SortOrder

Instead, fetch the config order on the frontend and pass a builtin
variant into the backend.
That makes the following unnecessary:
* Resolving the config sort in search/mod.rs
* Deserializing the Column enum
* Config accessors for the sort columns
This commit is contained in:
RumovZ 2021-04-10 11:13:42 +02:00
parent 5982a777aa
commit 801f52df40
7 changed files with 45 additions and 82 deletions

View file

@ -528,7 +528,7 @@ class Collection:
The reverse argument only applies when a BrowserColumns.Column is provided;
otherwise the collection config defines whether reverse is set or not.
"""
mode = _build_sort_mode(order, reverse)
mode = self._build_sort_mode(order, reverse, False)
return cast(
Sequence[CardId], self._backend.search_cards(search=query, order=mode)
)
@ -544,11 +544,38 @@ class Collection:
To programmatically construct a search string, see .build_search_string().
The order parameter is documented in .find_cards().
"""
mode = _build_sort_mode(order, reverse)
mode = self._build_sort_mode(order, reverse, True)
return cast(
Sequence[NoteId], self._backend.search_notes(search=query, order=mode)
)
def _build_sort_mode(
self,
order: Union[bool, str, BrowserColumns.Column],
reverse: bool,
finding_notes: bool,
) -> _pb.SortOrder:
if isinstance(order, str):
return _pb.SortOrder(custom=order)
if isinstance(order, bool):
if order is False:
return _pb.SortOrder(none=_pb.Empty())
# order=True: set args to sort column and reverse from config
sort_key = "noteSortType" if finding_notes else "sortType"
order = self.get_browser_column(self.get_config(sort_key))
reverse_key = (
Config.Bool.BROWSER_NOTE_SORT_BACKWARDS
if finding_notes
else Config.Bool.BROWSER_SORT_BACKWARDS
)
reverse = self.get_config_bool(reverse_key)
if isinstance(order, BrowserColumns.Column):
if order.sorting != BrowserColumns.SORTING_NONE:
return _pb.SortOrder(
builtin=_pb.SortOrder.Builtin(column=order.key, reverse=reverse)
)
raise InvalidInput(f"{order} is not a valid sort order.")
def find_and_replace(
self,
*,
@ -701,6 +728,12 @@ class Collection:
def all_browser_columns(self) -> Sequence[BrowserColumns.Column]:
return self._backend.all_browser_columns()
def get_browser_column(self, key: str) -> Optional[BrowserColumns.Column]:
for column in self._backend.all_browser_columns():
if column.key == key:
return column
return None
def browser_row_for_id(
self, id_: int
) -> Tuple[Generator[Tuple[str, bool], None, None], BrowserRow.Color.V, str, int]:
@ -1121,20 +1154,3 @@ class _ReviewsUndo:
_UndoInfo = Union[_ReviewsUndo, LegacyCheckpoint, None]
def _build_sort_mode(
order: Union[bool, str, BrowserColumns.Column],
reverse: bool,
) -> _pb.SortOrder:
if isinstance(order, str):
return _pb.SortOrder(custom=order)
if isinstance(order, bool):
if order is True:
return _pb.SortOrder(from_config=_pb.Empty())
return _pb.SortOrder(none=_pb.Empty())
if order.sorting != BrowserColumns.SORTING_NONE:
return _pb.SortOrder(
builtin=_pb.SortOrder.Builtin(column=order.key, reverse=reverse)
)
raise InvalidInput(f"{order} is not a valid sort order.")

View file

@ -124,13 +124,12 @@ def test_findCards():
col.set_config_bool(Config.Bool.BROWSER_SORT_BACKWARDS, True)
col.flush()
assert col.findCards("", order=True)[0] in latestCardIds
sort_columns = dict(((c.key, c) for c in col.all_browser_columns()))
assert (
col.find_cards("", order=sort_columns["cardDue"], reverse=False)[0]
col.find_cards("", order=col.get_browser_column("cardDue"), reverse=False)[0]
== firstCardId
)
assert (
col.find_cards("", order=sort_columns["cardDue"], reverse=True)[0]
col.find_cards("", order=col.get_browser_column("cardDue"), reverse=True)[0]
!= firstCardId
)
# model

View file

@ -799,10 +799,9 @@ message SortOrder {
bool reverse = 2;
}
oneof value {
Empty from_config = 1;
Empty none = 2;
string custom = 3;
Builtin builtin = 4;
Empty none = 1;
string custom = 2;
Builtin builtin = 3;
}
}

View file

@ -107,10 +107,9 @@ impl SearchService for Backend {
impl From<Option<SortOrderProto>> for SortMode {
fn from(order: Option<SortOrderProto>) -> Self {
use pb::sort_order::Value as V;
match order.unwrap_or(V::FromConfig(pb::Empty {})) {
match order.unwrap_or(V::None(pb::Empty {})) {
V::None(_) => SortMode::NoOrder,
V::Custom(s) => SortMode::Custom(s),
V::FromConfig(_) => SortMode::FromConfig,
V::Builtin(b) => SortMode::Builtin {
column: Column::from_str(&b.column).unwrap_or_default(),
reverse: b.reverse,

View file

@ -4,7 +4,6 @@
use std::sync::Arc;
use itertools::Itertools;
use serde::Deserialize;
use strum::{Display, EnumIter, EnumString, IntoEnumIterator};
use crate::error::{AnkiError, Result};
@ -23,46 +22,34 @@ use crate::{
timestamp::{TimestampMillis, TimestampSecs},
};
#[derive(Deserialize, Debug, PartialEq, Clone, Copy, Display, EnumIter, EnumString)]
#[serde(rename_all = "camelCase")]
#[derive(Debug, PartialEq, Clone, Copy, Display, EnumIter, EnumString)]
#[strum(serialize_all = "camelCase")]
pub enum Column {
#[serde(rename = "")]
#[strum(serialize = "")]
Custom,
Answer,
CardMod,
#[serde(rename = "template")]
#[strum(serialize = "template")]
Cards,
Deck,
#[serde(rename = "cardDue")]
#[strum(serialize = "cardDue")]
Due,
#[serde(rename = "cardEase")]
#[strum(serialize = "cardEase")]
Ease,
#[serde(rename = "cardLapses")]
#[strum(serialize = "cardLapses")]
Lapses,
#[serde(rename = "cardIvl")]
#[strum(serialize = "cardIvl")]
Interval,
#[serde(rename = "noteCrt")]
#[strum(serialize = "noteCrt")]
NoteCreation,
NoteMod,
#[serde(rename = "note")]
#[strum(serialize = "note")]
Notetype,
Question,
#[serde(rename = "cardReps")]
#[strum(serialize = "cardReps")]
Reps,
#[serde(rename = "noteFld")]
#[strum(serialize = "noteFld")]
SortField,
#[serde(rename = "noteTags")]
#[strum(serialize = "noteTags")]
Tags,
}

View file

@ -9,7 +9,6 @@ mod string;
pub(crate) mod undo;
pub use self::{bool::BoolKey, string::StringKey};
use crate::browser_table::Column;
use crate::prelude::*;
use serde::{de::DeserializeOwned, Serialize};
use serde_repr::{Deserialize_repr, Serialize_repr};
@ -46,10 +45,6 @@ pub(crate) enum ConfigKey {
#[strum(to_string = "timeLim")]
AnswerTimeLimitSecs,
#[strum(to_string = "sortType")]
BrowserSortColumn,
#[strum(to_string = "noteSortType")]
BrowserNoteSortColumn,
#[strum(to_string = "curDeck")]
CurrentDeckId,
#[strum(to_string = "curModel")]
@ -127,16 +122,6 @@ impl Collection {
Ok(())
}
pub(crate) fn get_browser_sort_column(&self) -> Column {
self.get_config_optional(ConfigKey::BrowserSortColumn)
.unwrap_or(Column::NoteCreation)
}
pub(crate) fn get_browser_note_sort_column(&self) -> Column {
self.get_config_optional(ConfigKey::BrowserNoteSortColumn)
.unwrap_or(Column::NoteCreation)
}
pub(crate) fn get_creation_utc_offset(&self) -> Option<i32> {
self.get_config_optional(ConfigKey::CreationOffset)
}
@ -269,7 +254,6 @@ pub(crate) enum Weekday {
#[cfg(test)]
mod test {
use super::*;
use crate::collection::open_test_collection;
use crate::decks::DeckId;
@ -277,7 +261,6 @@ mod test {
fn defaults() {
let col = open_test_collection();
assert_eq!(col.get_current_deck_id(), DeckId(1));
assert_eq!(col.get_browser_sort_column(), Column::SortField);
}
#[test]

View file

@ -14,8 +14,8 @@ use rusqlite::types::FromSql;
use std::borrow::Cow;
use crate::{
browser_table::Column, card::CardId, card::CardType, collection::Collection, config::BoolKey,
error::Result, notes::NoteId, prelude::AnkiError, search::parser::parse,
browser_table::Column, card::CardId, card::CardType, collection::Collection, error::Result,
notes::NoteId, prelude::AnkiError, search::parser::parse,
};
use sqlwriter::{RequiredTable, SqlWriter};
@ -28,7 +28,6 @@ pub enum ReturnItemType {
#[derive(Debug, PartialEq, Clone)]
pub enum SortMode {
NoOrder,
FromConfig,
Builtin { column: Column, reverse: bool },
Custom(String),
}
@ -62,7 +61,6 @@ impl SortMode {
fn required_table(&self) -> RequiredTable {
match self {
SortMode::NoOrder => RequiredTable::CardsOrNotes,
SortMode::FromConfig => unreachable!(),
SortMode::Builtin { column, .. } => column.required_table(),
SortMode::Custom(ref text) => {
if text.contains("n.") {
@ -94,13 +92,12 @@ impl Column {
}
impl Collection {
pub fn search<T>(&mut self, search: &str, mut mode: SortMode) -> Result<Vec<T>>
pub fn search<T>(&mut self, search: &str, mode: SortMode) -> Result<Vec<T>>
where
T: FromSql + AsReturnItemType,
{
let item_type = T::as_return_item_type();
let top_node = Node::Group(parse(search)?);
self.resolve_config_sort(item_type, &mut mode);
let writer = SqlWriter::new(self, item_type);
let (mut sql, args) = writer.build_query(&top_node, mode.required_table())?;
@ -130,7 +127,6 @@ impl Collection {
) -> Result<()> {
match mode {
SortMode::NoOrder => (),
SortMode::FromConfig => unreachable!(),
SortMode::Builtin { column, reverse } => {
prepare_sort(self, column, item_type)?;
sql.push_str(" order by ");
@ -173,22 +169,6 @@ impl Collection {
.execute(&args)
.map_err(Into::into)
}
/// If the sort mode is based on a config setting, look it up.
fn resolve_config_sort(&self, item_type: ReturnItemType, mode: &mut SortMode) {
if mode == &SortMode::FromConfig {
*mode = match item_type {
ReturnItemType::Cards => SortMode::Builtin {
column: self.get_browser_sort_column(),
reverse: self.get_bool(BoolKey::BrowserSortBackwards),
},
ReturnItemType::Notes => SortMode::Builtin {
column: self.get_browser_note_sort_column(),
reverse: self.get_bool(BoolKey::BrowserNoteSortBackwards),
},
}
}
}
}
/// Add the order clause to the sql.