shift keep-current-selection logic into sidebar's refresh()

By calling refresh() manually after performing an op, we were refreshing
twice, and the selection was being lost when changes were made outside
of the sidebar.

Also drop the after_hooks arg to perform_op(), since nothing is using
it now.

qt/aqt/main.py

@@ -771,7 +771,6 @@ class AnkiQt(QMainWindow):
         *,
         success: PerformOpOptionalSuccessCallback = None,
         failure: PerformOpOptionalFailureCallback = None,
-        after_hooks: Optional[Callable[[], None]] = None,
         handler: Optional[object] = None,
     ) -> None:
         """Run the provided operation on a background thread.
@@ -795,10 +794,6 @@ class AnkiQt(QMainWindow):
 
         If op() throws an exception, it will be shown in a popup, or
         passed to failure() if it is provided.
-
-        after_hooks() will be called after hooks are fired, if it is provided.
-        Components can use this to ignore change notices generated by operations
-        they invoke themselves, or perform some subsequent action.
         """
 
         self._increase_background_ops()
@@ -826,7 +821,7 @@ class AnkiQt(QMainWindow):
                 status = self.col.undo_status()
                 self._update_undo_actions_for_status_and_save(status)
                 # fire change hooks
-                self._fire_change_hooks_after_op_performed(result, after_hooks, handler)
+                self._fire_change_hooks_after_op_performed(result, handler)
 
         self.taskman.with_progress(op, wrapped_done)
 
@@ -844,7 +839,6 @@ class AnkiQt(QMainWindow):
     def _fire_change_hooks_after_op_performed(
         self,
         result: ResultWithChanges,
-        after_hooks: Optional[Callable[[], None]],
         handler: Optional[object],
     ) -> None:
         if isinstance(result, OpChanges):
@@ -859,8 +853,6 @@ class AnkiQt(QMainWindow):
         # fire legacy hook so old code notices changes
         if self.col.op_made_changes(changes):
             gui_hooks.state_did_reset()
-        if after_hooks:
-            after_hooks()
 
     def _synthesize_op_did_execute_from_reset(self) -> None:
         """Fire the `operation_did_execute` hook with everything marked as changed,

qt/aqt/operations/deck.py

@@ -3,7 +3,7 @@
 
 from __future__ import annotations
 
-from typing import Callable, Optional, Sequence
+from typing import Optional, Sequence
 
 from anki.decks import DeckCollapseScope, DeckId
 from aqt import AnkiQt, QWidget
@@ -41,10 +41,9 @@ def rename_deck(
     mw: AnkiQt,
     deck_id: DeckId,
     new_name: str,
-    after_rename: Callable[[], None] = None,
 ) -> None:
     mw.perform_op(
-        lambda: mw.col.decks.rename(deck_id, new_name), after_hooks=after_rename
+        lambda: mw.col.decks.rename(deck_id, new_name),
     )
 
 

qt/aqt/operations/tag.py

@@ -3,7 +3,7 @@
 
 from __future__ import annotations
 
-from typing import Callable, Optional, Sequence
+from typing import Optional, Sequence
 
 from anki.collection import OpChangesWithCount
 from anki.notes import NoteId
@@ -57,7 +57,6 @@ def rename_tag(
     parent: QWidget,
     current_name: str,
     new_name: str,
-    after_rename: Callable[[], None],
 ) -> None:
     def success(out: OpChangesWithCount) -> None:
         if out.count:
@@ -68,7 +67,6 @@ def rename_tag(
     mw.perform_op(
         lambda: mw.col.tags.rename(old=current_name, new=new_name),
         success=success,
-        after_hooks=after_rename,
     )
 
 

qt/aqt/sidebar.py

@@ -172,6 +172,18 @@ class SidebarItem:
         )
         return self._search_matches_self or self._search_matches_child
 
+    def has_same_id(self, other: SidebarItem) -> bool:
+        "True if `other` is same type, with same id/name."
+        if other.item_type == self.item_type:
+            if self.item_type == SidebarItemType.TAG:
+                return self.full_name == other.full_name
+            elif self.item_type == SidebarItemType.SAVED_SEARCH:
+                return self.name == other.name
+            else:
+                return other.id == self.id
+
+        return False
+
 
 class SidebarModel(QAbstractItemModel):
     def __init__(self, sidebar: SidebarTreeView, root: SidebarItem) -> None:
@@ -432,13 +444,16 @@ class SidebarTreeView(QTreeView):
             self.refresh()
             self._refresh_needed = False
 
-    def refresh(
+    def refresh(self) -> None:
-        self, is_current: Optional[Callable[[SidebarItem], bool]] = None
-    ) -> None:
         "Refresh list. No-op if sidebar is not visible."
         if not self.isVisible():
             return
 
+        if self.model() and (idx := self.currentIndex()):
+            current_item = self.model().item_for_index(idx)
+        else:
+            current_item = None
+
         def on_done(root: SidebarItem) -> None:
             # user may have closed browser
             if sip.isdeleted(self):
@@ -454,8 +469,8 @@ class SidebarTreeView(QTreeView):
                 self.search_for(self.current_search)
             else:
                 self._expand_where_necessary(model)
-            if is_current:
+            if current_item:
-                self.restore_current(is_current)
+                self.restore_current(current_item)
 
             self.setUpdatesEnabled(True)
 
@@ -464,8 +479,8 @@ class SidebarTreeView(QTreeView):
 
         self.mw.query_op(self._root_tree, success=on_done)
 
-    def restore_current(self, is_current: Callable[[SidebarItem], bool]) -> None:
+    def restore_current(self, current: SidebarItem) -> None:
-        if current := self.find_item(is_current):
+        if current := self.find_item(current.has_same_id):
             index = self.model().index_for_item(current)
             self.selectionModel().setCurrentIndex(
                 index, QItemSelectionModel.SelectCurrent
@@ -1165,21 +1180,21 @@ class SidebarTreeView(QTreeView):
     def rename_deck(self, item: SidebarItem, new_name: str) -> None:
         if not new_name:
             return
-        new_name = item.name_prefix + new_name
+
+        # update UI immediately, to avoid redraw
+        item.name = new_name
+
+        full_name = item.name_prefix + new_name
         deck_id = DeckId(item.id)
 
         def after_fetch(deck: Deck) -> None:
-            if new_name == deck.name:
+            if full_name == deck.name:
                 return
 
             rename_deck(
                 mw=self.mw,
                 deck_id=deck_id,
-                new_name=new_name,
+                new_name=full_name,
-                after_rename=lambda: self.refresh(
-                    lambda other: other.item_type == SidebarItemType.DECK
-                    and other.id == item.id
-                ),
             )
 
         self.mw.query_op(lambda: self.mw.col.get_deck(deck_id), success=after_fetch)
@@ -1208,16 +1223,13 @@ class SidebarTreeView(QTreeView):
         new_name = item.name_prefix + new_name
 
         item.name = new_name_base
+        item.full_name = new_name
 
         rename_tag(
             mw=self.mw,
             parent=self.browser,
             current_name=old_name,
             new_name=new_name,
-            after_rename=lambda: self.refresh(
-                lambda item: item.item_type == SidebarItemType.TAG
-                and item.full_name == new_name
-            ),
         )
 
     # Saved searches
@@ -1250,10 +1262,7 @@ class SidebarTreeView(QTreeView):
             return
         conf[name] = search
         self._set_saved_searches(conf)
-        self.refresh(
+        self.refresh()
-            lambda item: item.item_type == SidebarItemType.SAVED_SEARCH
-            and item.name == name
-        )
 
     def remove_saved_searches(self, _item: SidebarItem) -> None:
         selected = self._selected_saved_searches()
@@ -1277,10 +1286,8 @@ class SidebarTreeView(QTreeView):
         conf[new_name] = filt
         del conf[old_name]
         self._set_saved_searches(conf)
-        self.refresh(
+        item.name = new_name
-            lambda item: item.item_type == SidebarItemType.SAVED_SEARCH
+        self.refresh()
-            and item.name == new_name
-        )
 
     def save_current_search(self) -> None:
         if (search := self._get_current_search()) is None: