diff --git a/ftl/core/browsing.ftl b/ftl/core/browsing.ftl index f84bb3890..30b43c260 100644 --- a/ftl/core/browsing.ftl +++ b/ftl/core/browsing.ftl @@ -151,3 +151,8 @@ browsing-changed-new-position = [one] Changed position of { $count } new card. *[other] Changed position of { $count } new cards. } +browsing-reparented-decks = + { $count -> + [one] Renamed { $count } deck. + *[other] Renamed { $count } decks. + } diff --git a/ftl/core/undo.ftl b/ftl/core/undo.ftl index 89c92c6b5..61d60d945 100644 --- a/ftl/core/undo.ftl +++ b/ftl/core/undo.ftl @@ -19,5 +19,3 @@ undo-update-card = Update Card undo-update-deck = Update Deck undo-forget-card = Forget Card undo-set-flag = Set Flag -# when dragging/dropping tags and decks in the sidebar -undo-reparent = Change Parent diff --git a/pylib/anki/decks.py b/pylib/anki/decks.py index 16a1a60c1..2eb0971f8 100644 --- a/pylib/anki/decks.py +++ b/pylib/anki/decks.py @@ -271,11 +271,13 @@ 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 + def reparent( + self, deck_ids: Sequence[DeckID], new_parent: DeckID + ) -> OpChangesWithCount: + """Rename one or more source decks that were dropped on `new_parent`. + If new_parent is 0, decks will be placed at the top level.""" + return self.col._backend.reparent_decks( + deck_ids=deck_ids, new_parent=new_parent ) # legacy @@ -286,7 +288,7 @@ class DeckManager: onto = 0 else: onto = int(ontoDeckDid) - self.drag_drop_decks([int(draggedDeckDid)], onto) + self.reparent([int(draggedDeckDid)], onto) # Deck configurations ############################################################# diff --git a/qt/aqt/deck_ops.py b/qt/aqt/deck_ops.py index 60a70a49a..1722f69cb 100644 --- a/qt/aqt/deck_ops.py +++ b/qt/aqt/deck_ops.py @@ -5,6 +5,7 @@ from __future__ import annotations from typing import Sequence +from anki.decks import DeckID from anki.lang import TR from aqt import AnkiQt, QWidget from aqt.utils import tooltip, tr @@ -14,7 +15,7 @@ def remove_decks( *, mw: AnkiQt, parent: QWidget, - deck_ids: Sequence[int], + deck_ids: Sequence[DeckID], ) -> None: mw.perform_op( lambda: mw.col.decks.remove(deck_ids), @@ -22,3 +23,14 @@ def remove_decks( tr(TR.BROWSING_CARDS_DELETED, count=out.count), parent=parent ), ) + + +def reparent_decks( + *, mw: AnkiQt, parent: QWidget, deck_ids: Sequence[DeckID], new_parent: DeckID +) -> None: + mw.perform_op( + lambda: mw.col.decks.reparent(deck_ids=deck_ids, new_parent=new_parent), + success=lambda out: tooltip( + tr(TR.BROWSING_REPARENTED_DECKS, count=out.count), parent=parent + ), + ) diff --git a/qt/aqt/deckbrowser.py b/qt/aqt/deckbrowser.py index af6ca9cb4..9bea3204f 100644 --- a/qt/aqt/deckbrowser.py +++ b/qt/aqt/deckbrowser.py @@ -1,8 +1,8 @@ # Copyright: Ankitects Pty Ltd and contributors # License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html + from __future__ import annotations -from concurrent.futures import Future from copy import deepcopy from dataclasses import dataclass from typing import Any @@ -13,7 +13,7 @@ from anki.decks import DeckTreeNode from anki.errors import DeckIsFilteredError from anki.utils import intTime from aqt import AnkiQt, gui_hooks -from aqt.deck_ops import remove_decks +from aqt.deck_ops import remove_decks, reparent_decks from aqt.qt import * from aqt.sound import av_player from aqt.toolbar import BottomBar @@ -304,21 +304,7 @@ class DeckBrowser: self._renderPage(reuse=True) def _handle_drag_and_drop(self, source: int, target: int) -> None: - def process() -> None: - self.mw.col.decks.drag_drop_decks([source], target) - - def on_done(fut: Future) -> None: - try: - fut.result() - except Exception as e: - showWarning(str(e)) - return - - self.mw.update_undo_actions() - gui_hooks.sidebar_should_refresh_decks() - self.show() - - self.mw.taskman.with_progress(process, on_done) + reparent_decks(mw=self.mw, parent=self.mw, deck_ids=[source], new_parent=target) def _delete(self, did: int) -> None: remove_decks(mw=self.mw, parent=self.mw, deck_ids=[did]) diff --git a/qt/aqt/sidebar.py b/qt/aqt/sidebar.py index 757c090af..c4a415ced 100644 --- a/qt/aqt/sidebar.py +++ b/qt/aqt/sidebar.py @@ -3,7 +3,6 @@ from __future__ import annotations -from concurrent.futures import Future from enum import Enum, auto from typing import Dict, Iterable, List, Optional, Tuple, cast @@ -16,7 +15,7 @@ from anki.tags import TagTreeNode from anki.types import assert_exhaustive from aqt import colors, gui_hooks from aqt.clayout import CardLayout -from aqt.deck_ops import remove_decks +from aqt.deck_ops import remove_decks, reparent_decks from aqt.models import Models from aqt.qt import * from aqt.tag_ops import remove_tags_for_all_notes, rename_tag, reparent_tags @@ -604,30 +603,18 @@ class SidebarTreeView(QTreeView): def _handle_drag_drop_decks( self, sources: List[SidebarItem], target: SidebarItem ) -> bool: - source_ids = [ + deck_ids = [ source.id for source in sources if source.item_type == SidebarItemType.DECK ] - if not source_ids: + if not deck_ids: return False - def on_done(fut: Future) -> None: - self.browser.model.endReset() - try: - fut.result() - except Exception as e: - showWarning(str(e)) - return - self.refresh() - self.mw.deckBrowser.refresh() - self.mw.update_undo_actions() + new_parent = target.id - def on_save() -> None: - self.browser.model.beginReset() - self.mw.taskman.with_progress( - lambda: self.col.decks.drag_drop_decks(source_ids, target.id), on_done - ) + reparent_decks( + mw=self.mw, parent=self.browser, deck_ids=deck_ids, new_parent=new_parent + ) - self.browser.editor.call_after_note_saved(on_save) return True def _handle_drag_drop_tags( diff --git a/rslib/backend.proto b/rslib/backend.proto index 52240ea8b..9877dba61 100644 --- a/rslib/backend.proto +++ b/rslib/backend.proto @@ -140,7 +140,7 @@ service DecksService { rpc GetDeckNames(GetDeckNamesIn) returns (DeckNames); rpc NewDeckLegacy(Bool) returns (Json); rpc RemoveDecks(DeckIDs) returns (OpChangesWithCount); - rpc DragDropDecks(DragDropDecksIn) returns (OpChanges); + rpc ReparentDecks(ReparentDecksIn) returns (OpChangesWithCount); rpc RenameDeck(RenameDeckIn) returns (OpChanges); } @@ -1108,9 +1108,9 @@ message GetDeckNamesIn { bool include_filtered = 2; } -message DragDropDecksIn { - repeated int64 source_deck_ids = 1; - int64 target_deck_id = 2; +message ReparentDecksIn { + repeated int64 deck_ids = 1; + int64 new_parent = 2; } message NoteIsDuplicateOrEmptyOut { diff --git a/rslib/src/backend/decks.rs b/rslib/src/backend/decks.rs index 9cfb908f0..fe725f0c5 100644 --- a/rslib/src/backend/decks.rs +++ b/rslib/src/backend/decks.rs @@ -114,14 +114,14 @@ impl DecksService for Backend { .map(Into::into) } - fn drag_drop_decks(&self, input: pb::DragDropDecksIn) -> Result { - let source_dids: Vec<_> = input.source_deck_ids.into_iter().map(Into::into).collect(); - let target_did = if input.target_deck_id == 0 { + fn reparent_decks(&self, input: pb::ReparentDecksIn) -> Result { + let deck_ids: Vec<_> = input.deck_ids.into_iter().map(Into::into).collect(); + let new_parent = if input.new_parent == 0 { None } else { - Some(input.target_deck_id.into()) + Some(input.new_parent.into()) }; - self.with_col(|col| col.drag_drop_decks(&source_dids, target_did)) + self.with_col(|col| col.reparent_decks(&deck_ids, new_parent)) .map(Into::into) } diff --git a/rslib/src/decks/mod.rs b/rslib/src/decks/mod.rs index 7b12ea5bc..0e5a9afa5 100644 --- a/rslib/src/decks/mod.rs +++ b/rslib/src/decks/mod.rs @@ -230,21 +230,21 @@ pub(crate) fn immediate_parent_name(machine_name: &str) -> Option<&str> { /// 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. +/// The returned name should be used to rename `dragged`. /// Arguments are expected in 'machine' form with an \x1f separator. -pub(crate) fn drag_drop_deck_name(dragged: &str, dropped: Option<&str>) -> String { +pub(crate) fn reparented_name(dragged: &str, dropped: Option<&str>) -> Option { 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() + None } else { // foo::bar onto baz -> baz::bar - format!("{}\x1f{}", dropped, dragged_base) + Some(format!("{}\x1f{}", dropped, dragged_base)) } } else { // foo::bar onto top level -> bar - dragged_base.into() + Some(dragged_base.into()) } } @@ -618,59 +618,64 @@ impl Collection { self.update_single_deck_undoable(deck, original) } - pub fn drag_drop_decks( + pub fn reparent_decks( &mut self, - source_decks: &[DeckID], - target: Option, - ) -> Result> { - let usn = self.usn()?; - self.transact(Op::RenameDeck, |col| { - let target_deck; - let mut target_name = None; - if let Some(target) = target { - if let Some(target) = col.storage.get_deck(target)? { - if target.is_filtered() { - return Err(AnkiError::DeckIsFiltered); - } - target_deck = target; - target_name = Some(target_deck.name.as_str()); - } - } + deck_ids: &[DeckID], + new_parent: Option, + ) -> Result> { + self.transact(Op::ReparentDeck, |col| { + col.reparent_decks_inner(deck_ids, new_parent) + }) + } - for source in source_decks { - if let Some(mut source) = col.storage.get_deck(*source)? { - let new_name = drag_drop_deck_name(&source.name, target_name); - if new_name == source.name { - continue; - } - let orig = source.clone(); + pub fn reparent_decks_inner( + &mut self, + deck_ids: &[DeckID], + new_parent: Option, + ) -> Result { + let usn = self.usn()?; + let target_deck; + let mut target_name = None; + if let Some(target) = new_parent { + if let Some(target) = self.storage.get_deck(target)? { + if target.is_filtered() { + return Err(AnkiError::DeckIsFiltered); + } + target_deck = target; + target_name = Some(target_deck.name.as_str()); + } + } + + let mut count = 0; + for deck in deck_ids { + if let Some(mut deck) = self.storage.get_deck(*deck)? { + if let Some(new_name) = reparented_name(&deck.name, target_name) { + count += 1; + let orig = deck.clone(); // this is basically update_deck_inner(), except: // - we skip the normalization in prepare_for_update() // - we skip the match_or_create_parents() step + // - we skip the final create_missing_parents(), as we don't allow parent->child + // renames - source.set_modified(usn); - source.name = new_name; - col.ensure_deck_name_unique(&mut source, usn)?; - col.rename_child_decks(&orig, &source.name, usn)?; - col.update_single_deck_undoable(&mut source, orig)?; - - // after updating, we need to ensure all grandparents exist, which may not be the case - // in the parent->child case - // FIXME: maybe we only need to do this once at the end of the loop? - col.create_missing_parents(&source.name, usn)?; + deck.set_modified(usn); + deck.name = new_name; + self.ensure_deck_name_unique(&mut deck, usn)?; + self.rename_child_decks(&orig, &deck.name, usn)?; + self.update_single_deck_undoable(&mut deck, orig)?; } } + } - Ok(()) - }) + Ok(count) } } #[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::decks::reparented_name; use crate::{ collection::{open_test_collection, Collection}, err::Result, @@ -843,32 +848,31 @@ mod test { 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"); + fn n_opt(s: &str) -> Option { + Some(n(s)) + } + + assert_eq!(reparented_name("drag", Some("drop")), n_opt("drop:drag")); + assert_eq!(reparented_name("drag", None), n_opt("drag")); + assert_eq!(reparented_name(&n("drag:child"), None), n_opt("child")); assert_eq!( - drag_drop_deck_name(&n("drag:child"), Some(&n("drop:deck"))), - n("drop:deck:child") + reparented_name(&n("drag:child"), Some(&n("drop:deck"))), + n_opt("drop:deck:child") ); assert_eq!( - drag_drop_deck_name(&n("drag:child"), Some("drag")), - n("drag:child") + reparented_name(&n("drag:child"), Some("drag")), + n_opt("drag:child") ); assert_eq!( - drag_drop_deck_name(&n("drag:child:grandchild"), Some("drag")), - n("drag:grandchild") + reparented_name(&n("drag:child:grandchild"), Some("drag")), + n_opt("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 + // drops to child not supported assert_eq!( - drag_drop_deck_name(&n("drag"), Some(&n("drag:child:grandchild"))), - n("drag") + reparented_name(&n("drag"), Some(&n("drag:child:grandchild"))), + None ); // 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") - ); + assert_eq!(reparented_name(&n("foo:bar"), Some(&n("foo:bar"))), None); } } diff --git a/rslib/src/ops.rs b/rslib/src/ops.rs index c0bad2827..a8543f0a8 100644 --- a/rslib/src/ops.rs +++ b/rslib/src/ops.rs @@ -15,6 +15,7 @@ pub enum Op { RemoveNote, RemoveTag, RenameDeck, + ReparentDeck, RenameTag, ReparentTag, ScheduleAsNew, @@ -57,7 +58,8 @@ impl Op { Op::SortCards => TR::BrowsingReschedule, Op::RenameTag => TR::ActionsRenameTag, Op::RemoveTag => TR::ActionsRemoveTag, - Op::ReparentTag => TR::UndoReparent, + Op::ReparentTag => TR::ActionsRenameTag, + Op::ReparentDeck => TR::ActionsRenameDeck, }; i18n.tr(key).to_string()