diff --git a/pylib/anki/decks.py b/pylib/anki/decks.py index 0c71ceeac..d3acfe4dc 100644 --- a/pylib/anki/decks.py +++ b/pylib/anki/decks.py @@ -39,6 +39,8 @@ DeckConfig = Union[FilteredDeck, Config] """ New/lrn/rev conf, from deck config""" QueueConfig = Dict[str, Any] +DeckID = int + class DecksDictProxy: def __init__(self, col: anki.collection.Collection): @@ -260,43 +262,22 @@ class DeckManager: # Drag/drop ############################################################# + def drag_drop_decks(self, source_decks: List[DeckID], target_deck: DeckID) -> None: + """Rename one or more source decks that were dropped on `target_deck`. + If target_deck is 0, decks will be placed at the top level.""" + self.col.backend.drag_drop_decks( + source_deck_ids=source_decks, target_deck_id=target_deck + ) + + # legacy def renameForDragAndDrop( - self, draggedDeckDid: int, ontoDeckDid: Optional[Union[int, str]] + self, draggedDeckDid: Union[int, str], ontoDeckDid: Optional[Union[int, str]] ) -> None: - draggedDeck = self.get(draggedDeckDid) - draggedDeckName = draggedDeck["name"] - ontoDeckName = self.get(ontoDeckDid)["name"] - - if ontoDeckDid is None or ontoDeckDid == "": - if len(self.path(draggedDeckName)) > 1: - self.rename(draggedDeck, self.basename(draggedDeckName)) - elif self._canDragAndDrop(draggedDeckName, ontoDeckName): - draggedDeck = self.get(draggedDeckDid) - draggedDeckName = draggedDeck["name"] - ontoDeckName = self.get(ontoDeckDid)["name"] - assert ontoDeckName.strip() - self.rename( - draggedDeck, ontoDeckName + "::" + self.basename(draggedDeckName) - ) - - def _canDragAndDrop(self, draggedDeckName: str, ontoDeckName: str) -> bool: - if ( - draggedDeckName == ontoDeckName - or self._isParent(ontoDeckName, draggedDeckName) - or self._isAncestor(draggedDeckName, ontoDeckName) - ): - return False + if not ontoDeckDid: + onto = 0 else: - return True - - def _isParent(self, parentDeckName: str, childDeckName: str) -> bool: - return self.path(childDeckName) == self.path(parentDeckName) + [ - self.basename(childDeckName) - ] - - def _isAncestor(self, ancestorDeckName: str, descendantDeckName: str) -> bool: - ancestorPath = self.path(ancestorDeckName) - return ancestorPath == self.path(descendantDeckName)[0 : len(ancestorPath)] + onto = int(ontoDeckDid) + self.drag_drop_decks([int(draggedDeckDid)], onto) # Deck configurations ############################################################# diff --git a/pylib/tests/test_decks.py b/pylib/tests/test_decks.py index 59fb8e130..74f6e5589 100644 --- a/pylib/tests/test_decks.py +++ b/pylib/tests/test_decks.py @@ -106,15 +106,15 @@ def test_renameForDragAndDrop(): col.decks.renameForDragAndDrop(chinese_did, languages_did) assert deckNames() == ["Languages", "Languages::Chinese", "Languages::Chinese::HSK"] - # Dragging a col onto itself is a no-op + # Dragging a deck onto itself is a no-op col.decks.renameForDragAndDrop(languages_did, languages_did) assert deckNames() == ["Languages", "Languages::Chinese", "Languages::Chinese::HSK"] - # Dragging a col onto its parent is a no-op + # Dragging a deck onto its parent is a no-op col.decks.renameForDragAndDrop(hsk_did, chinese_did) assert deckNames() == ["Languages", "Languages::Chinese", "Languages::Chinese::HSK"] - # Dragging a col onto a descendant is a no-op + # Dragging a deck onto a descendant is a no-op col.decks.renameForDragAndDrop(languages_did, hsk_did) assert deckNames() == ["Languages", "Languages::Chinese", "Languages::Chinese::HSK"] @@ -122,11 +122,11 @@ def test_renameForDragAndDrop(): col.decks.renameForDragAndDrop(hsk_did, languages_did) assert deckNames() == ["Languages", "Languages::Chinese", "Languages::HSK"] - # Can drag a col onto its sibling + # Can drag a deck onto its sibling col.decks.renameForDragAndDrop(hsk_did, chinese_did) assert deckNames() == ["Languages", "Languages::Chinese", "Languages::Chinese::HSK"] - # Can drag a col back to the top level + # Can drag a deck back to the top level col.decks.renameForDragAndDrop(chinese_did, None) assert deckNames() == ["Chinese", "Chinese::HSK", "Languages"] diff --git a/qt/aqt/deckbrowser.py b/qt/aqt/deckbrowser.py index 3b0b16666..b585ddfaf 100644 --- a/qt/aqt/deckbrowser.py +++ b/qt/aqt/deckbrowser.py @@ -6,6 +6,7 @@ from __future__ import annotations from concurrent.futures import Future from copy import deepcopy from dataclasses import dataclass +from typing import Any import aqt from anki.errors import DeckRenameError @@ -63,7 +64,7 @@ class DeckBrowser: # Event handlers ########################################################################## - def _linkHandler(self, url): + def _linkHandler(self, url: str) -> Any: if ":" in url: (cmd, arg) = url.split(":", 1) else: @@ -83,8 +84,8 @@ class DeckBrowser: gui_hooks.sidebar_should_refresh_decks() self.refresh() elif cmd == "drag": - draggedDeckDid, ontoDeckDid = arg.split(",") - self._dragDeckOnto(draggedDeckDid, ontoDeckDid) + source, target = arg.split(",") + self._handle_drag_and_drop(int(source), int(target or 0)) elif cmd == "collapse": self._collapse(int(arg)) return False @@ -271,13 +272,9 @@ class DeckBrowser: node.collapsed = not node.collapsed self._renderPage(reuse=True) - def _dragDeckOnto(self, draggedDeckDid: int, ontoDeckDid: int): - try: - self.mw.col.decks.renameForDragAndDrop(draggedDeckDid, ontoDeckDid) - gui_hooks.sidebar_should_refresh_decks() - except DeckRenameError as e: - return showWarning(e.description) - + def _handle_drag_and_drop(self, source: int, target: int) -> None: + self.mw.col.decks.drag_drop_decks([source], target) + gui_hooks.sidebar_should_refresh_decks() self.show() def ask_delete_deck(self, did: int) -> bool: diff --git a/rslib/backend.proto b/rslib/backend.proto index 1a7bbc88e..1488f1f61 100644 --- a/rslib/backend.proto +++ b/rslib/backend.proto @@ -138,6 +138,7 @@ service BackendService { rpc GetDeckNames(GetDeckNamesIn) returns (DeckNames); rpc NewDeckLegacy(Bool) returns (Json); rpc RemoveDeck(DeckID) returns (Empty); + rpc DragDropDecks(DragDropDecksIn) returns (Empty); // deck config @@ -991,6 +992,11 @@ message GetDeckNamesIn { bool include_filtered = 2; } +message DragDropDecksIn { + repeated int64 source_deck_ids = 1; + int64 target_deck_id = 2; +} + message NoteIsDuplicateOrEmptyOut { enum State { NORMAL = 0; diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index 2faea8785..aeba5294e 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -792,6 +792,17 @@ impl BackendService for Backend { .map(Into::into) } + fn drag_drop_decks(&self, input: pb::DragDropDecksIn) -> BackendResult { + let source_dids: Vec<_> = input.source_deck_ids.into_iter().map(Into::into).collect(); + let target_did = if input.target_deck_id == 0 { + None + } else { + Some(input.target_deck_id.into()) + }; + self.with_col(|col| col.drag_drop_decks(&source_dids, target_did)) + .map(Into::into) + } + // deck config //---------------------------------------------------- diff --git a/rslib/src/decks/mod.rs b/rslib/src/decks/mod.rs index 27f7d1e70..707193534 100644 --- a/rslib/src/decks/mod.rs +++ b/rslib/src/decks/mod.rs @@ -175,6 +175,26 @@ pub(crate) fn immediate_parent_name(machine_name: &str) -> Option<&str> { machine_name.rsplitn(2, '\x1f').nth(1) } +/// Determine name to rename a deck to, when `dragged` is dropped on `dropped`. +/// `dropped` being unset represents a drop at the top or bottom of the deck list. +/// The returned name should be used to rename `dragged`, and may be unchanged. +/// Arguments are expected in 'machine' form with an \x1f separator. +pub(crate) fn drag_drop_deck_name(dragged: &str, dropped: Option<&str>) -> String { + let dragged_base = dragged.rsplit('\x1f').next().unwrap(); + if let Some(dropped) = dropped { + if dropped.starts_with(dragged) { + // foo onto foo::bar, or foo onto itself -> no-op + dragged.to_string() + } else { + // foo::bar onto baz -> baz::bar + format!("{}\x1f{}", dropped, dragged_base) + } + } else { + // foo::bar onto top level -> bar + dragged_base.into() + } +} + impl Collection { pub(crate) fn default_deck_is_empty(&self) -> Result { self.storage.deck_is_empty(DeckID(1)) @@ -522,11 +542,51 @@ impl Collection { deck.set_modified(usn); self.add_or_update_single_deck(deck, usn) } + + pub fn drag_drop_decks( + &mut self, + source_decks: &[DeckID], + target: Option, + ) -> Result<()> { + self.state.deck_cache.clear(); + let usn = self.usn()?; + self.transact(None, |col| { + let target_deck; + let mut target_name = None; + if let Some(target) = target { + if let Some(target) = col.storage.get_deck(target)? { + target_deck = target; + target_name = Some(target_deck.name.as_str()); + } + } + + for source in source_decks { + if let Some(mut source) = col.storage.get_deck(*source)? { + let orig = source.clone(); + let new_name = drag_drop_deck_name(&source.name, target_name); + if new_name == source.name { + continue; + } + source.name = new_name; + col.ensure_deck_name_unique(&mut source, usn)?; + col.rename_child_decks(&orig, &source.name, usn)?; + source.set_modified(usn); + col.storage.update_deck(&source)?; + // after updating, we need to ensure all grandparents exist, which may not be the case + // in the parent->child case + col.create_missing_parents(&source.name, usn)?; + } + } + + Ok(()) + }) + } } #[cfg(test)] mod test { use super::{human_deck_name_to_native, immediate_parent_name, normalize_native_name}; + use crate::decks::drag_drop_deck_name; use crate::{ collection::{open_test_collection, Collection}, err::Result, @@ -675,4 +735,39 @@ mod test { Ok(()) } + + #[test] + fn drag_drop() { + // use custom separator to make the tests easier to read + fn n(s: &str) -> String { + s.replace(":", "\x1f") + } + assert_eq!(drag_drop_deck_name("drag", Some("drop")), n("drop:drag")); + assert_eq!(&drag_drop_deck_name("drag", None), "drag"); + assert_eq!(&drag_drop_deck_name(&n("drag:child"), None), "child"); + assert_eq!( + drag_drop_deck_name(&n("drag:child"), Some(&n("drop:deck"))), + n("drop:deck:child") + ); + assert_eq!( + drag_drop_deck_name(&n("drag:child"), Some("drag")), + n("drag:child") + ); + assert_eq!( + drag_drop_deck_name(&n("drag:child:grandchild"), Some("drag")), + n("drag:grandchild") + ); + // while the renaming code should be able to cope with renaming a parent to a child, + // it's not often useful and can be difficult for the user to clean up if done accidentally, + // so it should be a no-op + assert_eq!( + drag_drop_deck_name(&n("drag"), Some(&n("drag:child:grandchild"))), + n("drag") + ); + // name doesn't change when deck dropped on itself + assert_eq!( + drag_drop_deck_name(&n("foo:bar"), Some(&n("foo:bar"))), + n("foo:bar") + ); + } }