From 3921e20cc50960cb385d2478ab2e7b75eb074c08 Mon Sep 17 00:00:00 2001 From: olivers3uiuc Date: Thu, 18 Jun 2026 17:32:55 -0400 Subject: [PATCH 1/3] More keyboard implements to streamline navigation of GUI. F2, Right, and Return/Enter to edit current param's value. Backspace to clear current param's value. Tab/Shift+Tab to shift focus to next/prev param (item level). Tab/Shift+Tab to commit changes to value and shift focus to next/prev param (editor level). Escape to cancel edits to a param's value. Param focus is now also updated when a user selects the editor. --- src/instrumentserver/gui/base_instrument.py | 26 ++++ src/instrumentserver/gui/instruments.py | 124 +++++++++++++++++++- src/instrumentserver/gui/shortcuts.py | 1 - 3 files changed, 147 insertions(+), 4 deletions(-) diff --git a/src/instrumentserver/gui/base_instrument.py b/src/instrumentserver/gui/base_instrument.py index 2e28793..f0c72f8 100644 --- a/src/instrumentserver/gui/base_instrument.py +++ b/src/instrumentserver/gui/base_instrument.py @@ -534,6 +534,14 @@ class InstrumentTreeViewBase(QtWidgets.QTreeView): #: emitted when this item got its star action triggered. itemStarToggle = QtCore.Signal(ItemBase) + #: Signal() + #: emitted when the user presses Enter, F2, or Right to enter edit mode on the selected parameter. + editCurrentParameter = QtCore.Signal() + + #: Signal() + #: emitted when the user presses Delete to clear the selected parameter's value. + clearCurrentParameter = QtCore.Signal() + def __init__( self, model: QtCore.QAbstractItemModel, @@ -586,6 +594,15 @@ def __init__( self.setContextMenuPolicy(QtCore.Qt.ContextMenuPolicy.CustomContextMenu) self.customContextMenuRequested.connect(self.onContextMenuRequested) + for key in ("Return", "Enter", "F2", "Right"): + sc = QtWidgets.QShortcut(QtGui.QKeySequence(key), self) + sc.setContext(QtCore.Qt.ShortcutContext.WidgetShortcut) + sc.activated.connect(self.editCurrentParameter) + + sc = QtWidgets.QShortcut(QtGui.QKeySequence("Backspace"), self) + sc.setContext(QtCore.Qt.ShortcutContext.WidgetShortcut) + sc.activated.connect(self.clearCurrentParameter) + @QtCore.Slot() def fillCollapsedDict(self, parentItem: Optional[ItemBase] = None) -> None: """ @@ -749,6 +766,15 @@ def onContextMenuRequested(self, pos: QtCore.QPoint) -> None: self.contextMenu.exec_(self.mapToGlobal(pos)) + def focusNextPrevChild(self, next: bool) -> bool: + current = self.currentIndex() + if current.isValid(): + next_idx = self.indexBelow(current) if next else self.indexAbove(current) + if next_idx.isValid(): + self.setCurrentIndex(next_idx) + return True + return super().focusNextPrevChild(next) + @QtCore.Slot() def onStarActionTrigger(self) -> None: self.itemStarToggle.emit(self.lastSelectedItem) diff --git a/src/instrumentserver/gui/instruments.py b/src/instrumentserver/gui/instruments.py index b96805a..66be25f 100644 --- a/src/instrumentserver/gui/instruments.py +++ b/src/instrumentserver/gui/instruments.py @@ -274,7 +274,6 @@ def __init__(self, unit: str = "", **kwargs: Any) -> None: self.unit = unit - class ParameterDelegate(DelegateBase): """ The delegate for the InstrumentParameters widget. @@ -286,6 +285,7 @@ def __init__(self, parent: Optional[QtCore.QObject] = None) -> None: # Stores as key the name of the item and as value the widget that the delegate creates. # used to keep a reference to the widget. self.parameters: Dict[str, QtWidgets.QWidget] = {} + self.navFilter: Optional["ValueCellNavigationFilter"] = None def createEditor( # type: ignore[override] self, @@ -301,6 +301,15 @@ def createEditor( # type: ignore[override] ret = ParameterWidget(element, widget) self.parameters[item.name] = ret # type: ignore[attr-defined] + + if self.navFilter is not None: + if isinstance(ret.paramWidget, AnyInput): + input_widget = ret.paramWidget.input + else: + input_widget = ret.paramWidget + input_widget.installEventFilter(self.navFilter) + self.navFilter.registerWidget(input_widget, index) + # Try to fetch and display current value immediately # ---- Chao: removed because the constructor of ParameterWidget object already calls parameter get ---- # if element.gettable: @@ -312,6 +321,76 @@ def createEditor( # type: ignore[override] return ret +class ValueCellNavigationFilter(QtCore.QObject): + """Event filter installed on value input widgets to handle Escape, Tab, and Shift+Tab.""" + + def __init__(self, treeView: InstrumentTreeViewBase) -> None: + super().__init__(treeView) + self._treeView = treeView + self._widgetIndex: Dict[QtCore.QObject, QtCore.QPersistentModelIndex] = {} + + def registerWidget(self, widget: QtCore.QObject, index: QtCore.QModelIndex) -> None: + self._widgetIndex[widget] = QtCore.QPersistentModelIndex(index) + + def eventFilter(self, obj: QtCore.QObject, event: QtCore.QEvent) -> bool: + if event.type() == QtCore.QEvent.Type.FocusIn: + if obj in self._widgetIndex: + idx = self._widgetIndex[obj] + if idx.isValid(): + self._treeView.setCurrentIndex(QtCore.QModelIndex(idx)) + return False + + if event.type() == QtCore.QEvent.Type.KeyPress: + assert isinstance(event, QtGui.QKeyEvent) + key = QtGui.QKeySequence(event.key()).toString() + + if key == "Esc": + host = self._findHostWidget(obj) + if isinstance(host, ParameterWidget): + host.setWidgetFromParameter() + elif isinstance(host, MethodDisplay): + host.anyInput.input.clear() + self._treeView.setFocus() + return True + + elif key == "Tab": + self._commitAndMove(obj, 1) + return True + + elif key == "Backtab": + self._commitAndMove(obj, -1) + return True + + return super().eventFilter(obj, event) + + def _findHostWidget( + self, obj: QtCore.QObject + ) -> Optional[Union[ParameterWidget, MethodDisplay]]: + parent = obj.parent() + while parent is not None: + if isinstance(parent, (ParameterWidget, MethodDisplay)): + return parent + parent = parent.parent() + return None + + def _commitAndMove(self, obj: QtCore.QObject, direction: int) -> None: + host = self._findHostWidget(obj) + if isinstance(host, ParameterWidget): + host.setButton.click() + elif isinstance(host, MethodDisplay): + host.runFun() + current = self._treeView.currentIndex() + if current.isValid(): + next_idx = ( + self._treeView.indexBelow(current) + if direction > 0 + else self._treeView.indexAbove(current) + ) + if next_idx.isValid(): + self._treeView.setCurrentIndex(next_idx) + self._treeView.setFocus() + + class ModelParameters(InstrumentModelBase): # : Signal(item, object) : Emitted when an item in the model has received a new value, first object is the item's # name, second object is its new value @@ -424,6 +503,7 @@ def __init__( super().__init__(model, [2], *args, **kwargs) self.delegate = ParameterDelegate(self) + self.delegate.navFilter = ValueCellNavigationFilter(self) self.setItemDelegateForColumn(2, self.delegate) self.setAllDelegatesPersistent() @@ -479,6 +559,10 @@ def __init__( **modelKwargs, ) + self.view.editCurrentParameter.connect(self._focusToParameterValue) + self.view.clearCurrentParameter.connect(self._clearCurrentParameter) + + def connectSignals(self) -> None: super().connectSignals() self.model.itemNewValue.connect(self.view.onItemNewValue) @@ -486,7 +570,6 @@ def connectSignals(self) -> None: self.shortcutManager.register( "toggle_python", self._togglePythonCurrentItem, self ) - self.shortcutManager.register("edit_value", self._focusToParameterValue, self) def _withCurrentParameter( self, callback: Callable[["ParameterWidget"], None] @@ -520,6 +603,19 @@ def _focusToParameterValue(self) -> None: else w.paramWidget.setFocus() ) ) + + @QtCore.Slot() + def _clearCurrentParameter(self) -> None: + self._withCurrentParameter( + lambda w: ( + w.paramWidget.input.clear() + if isinstance(w.paramWidget, AnyInput) + else w.paramWidget.clear() + if hasattr(w.paramWidget, 'clear') + else None + ) + ) + # ----------------- Parameters Display Classes - Ending -------------------------------- @@ -545,6 +641,14 @@ def createEditor( # type: ignore[override] ret = ParameterWidget(parameter=element, parent=widget, additionalWidgets=[rw]) self.parameters[item.name] = ret # type: ignore[attr-defined] + if self.navFilter is not None: + if isinstance(ret.paramWidget, AnyInput): + input_widget = ret.paramWidget.input + else: + input_widget = ret.paramWidget + input_widget.installEventFilter(self.navFilter) + self.navFilter.registerWidget(input_widget, index) + return ret def makeRemoveWidget( @@ -572,6 +676,7 @@ def __init__( super().__init__(model, [2], *args, **kwargs) self.delegate = ParameterDeleteDelegate(self) + self.delegate.navFilter = ValueCellNavigationFilter(self) self.setItemDelegateForColumn(2, self.delegate) self.setAllDelegatesPersistent() @@ -769,6 +874,7 @@ def __init__(self, parent: Optional[QtCore.QObject] = None) -> None: super().__init__(parent=parent) self.methods: Dict[str, "MethodDisplay"] = {} + self.navFilter: Optional[ValueCellNavigationFilter] = None def createEditor( # type: ignore[override] self, @@ -786,6 +892,11 @@ def createEditor( # type: ignore[override] parent.clearAlertsAction.triggered.connect(ret.alertLabel.clearAlert) # type: ignore[union-attr] self.methods[item.name] = ret # type: ignore[attr-defined] + + if self.navFilter is not None: + ret.anyInput.input.installEventFilter(self.navFilter) + self.navFilter.registerWidget(ret.anyInput.input, index) + return ret @@ -804,6 +915,7 @@ def __init__( self.contextMenu.addAction(self.clearAlertsAction) self.delegate = MethodsDelegate(self) + self.delegate.navFilter = ValueCellNavigationFilter(self) self.setItemDelegateForColumn(1, self.delegate) self.setAllDelegatesPersistent() @@ -832,12 +944,14 @@ def __init__(self, instrument: Any, **kwargs: Any) -> None: **modelKwargs, ) + self.view.editCurrentParameter.connect(self._focusToMethodValue) + self.view.clearCurrentParameter.connect(self._clearCurrentMethod) + def connectSignals(self) -> None: super().connectSignals() self.shortcutManager.register( "toggle_python", self._togglePythonCurrentItem, self ) - self.shortcutManager.register("edit_value", self._focusToMethodValue, self) self.shortcutManager.register("run_method", self._runCurrentMethod, self) def _withCurrentMethod(self, callback: Callable[["MethodDisplay"], None]) -> None: @@ -855,6 +969,10 @@ def _togglePythonCurrentItem(self) -> None: def _focusToMethodValue(self) -> None: self._withCurrentMethod(lambda w: w.anyInput.input.setFocus()) + @QtCore.Slot() + def _clearCurrentMethod(self) -> None: + self._withCurrentMethod(lambda w: w.anyInput.input.clear()) + @QtCore.Slot() def _runCurrentMethod(self) -> None: self._withCurrentMethod(lambda w: w.runFun()) diff --git a/src/instrumentserver/gui/shortcuts.py b/src/instrumentserver/gui/shortcuts.py index c528a6a..77c8096 100644 --- a/src/instrumentserver/gui/shortcuts.py +++ b/src/instrumentserver/gui/shortcuts.py @@ -43,7 +43,6 @@ class KeyboardShortcutManager: "save_items": ("Ctrl+Shift+S", "Save parameters to JSON file"), "fit_column": ("Ctrl+Shift+D", "Fits column width"), "sort_column": ("Ctrl+D", "Toggle sorting of selected column"), - "edit_value": ("Right", "Jump cursor to value field for selected parameter"), } def __init__(self) -> None: From 2a0ffe30da461696bef5a517ae954e85ae42cf2e Mon Sep 17 00:00:00 2001 From: olivers3uiuc Date: Fri, 26 Jun 2026 17:03:18 -0400 Subject: [PATCH 2/3] Undo/redo implemented using Qt's built-in undo stack. Undoable actions: set value, star/trash param, toggle eval, delete param --- src/instrumentserver/gui/base_instrument.py | 47 +++-- src/instrumentserver/gui/instruments.py | 114 +++++++++-- src/instrumentserver/gui/parameters.py | 31 +++ src/instrumentserver/gui/shortcuts.py | 2 + src/instrumentserver/gui/undo_commands.py | 200 ++++++++++++++++++++ 5 files changed, 362 insertions(+), 32 deletions(-) create mode 100644 src/instrumentserver/gui/undo_commands.py diff --git a/src/instrumentserver/gui/base_instrument.py b/src/instrumentserver/gui/base_instrument.py index f0c72f8..561c6bd 100644 --- a/src/instrumentserver/gui/base_instrument.py +++ b/src/instrumentserver/gui/base_instrument.py @@ -109,6 +109,8 @@ from instrumentserver import QtCore, QtGui, QtWidgets from instrumentserver.gui.shortcuts import KeyboardShortcutManager +from .undo_commands import ToggleStarTrashCommand + class ItemBase(QtGui.QStandardItem): """ @@ -767,14 +769,14 @@ def onContextMenuRequested(self, pos: QtCore.QPoint) -> None: self.contextMenu.exec_(self.mapToGlobal(pos)) def focusNextPrevChild(self, next: bool) -> bool: - current = self.currentIndex() - if current.isValid(): - next_idx = self.indexBelow(current) if next else self.indexAbove(current) - if next_idx.isValid(): - self.setCurrentIndex(next_idx) - return True - return super().focusNextPrevChild(next) - + current = self.currentIndex() + if current.isValid(): + next_idx = self.indexBelow(current) if next else self.indexAbove(current) + if next_idx.isValid(): + self.setCurrentIndex(next_idx) + return True + return super().focusNextPrevChild(next) + @QtCore.Slot() def onStarActionTrigger(self) -> None: self.itemStarToggle.emit(self.lastSelectedItem) @@ -842,6 +844,8 @@ def __init__( self.layout_.addWidget(self.view) self.setLayout(self.layout_) + self.undoStack: Any = None + self.view.expandAll() if callSignals: @@ -857,8 +861,12 @@ def connectSignals(self) -> None: self.proxyModel.filterIncoming.connect(self.view.fillCollapsedDict) self.proxyModel.filterFinished.connect(self.view.restoreCollapsedDict) - self.view.itemStarToggle.connect(self.model.onItemStarToggle) - self.view.itemTrashToggle.connect(self.model.onItemTrashToggle) + self.view.itemStarToggle.connect( + lambda item: self._toggleStarTrash("star", item) + ) + self.view.itemTrashToggle.connect( + lambda item: self._toggleStarTrash("trash", item) + ) self.lineEdit.textChanged.connect(self.proxyModel.onTextFilterChange) @@ -954,13 +962,28 @@ def _toggleCurrentItem(self, signal: QtCore.SignalInstance) -> None: self.view.lastSelectedItem = item signal.emit(item) + def _toggleStarTrash(self, mode: str, item: Optional["ItemBase"] = None) -> None: + if item is None: + item = self._getCurrentItem() + if item is None: + return + if self.undoStack is not None: + cmd = ToggleStarTrashCommand(item, self.model, mode) + self.view.lastSelectedItem = item + if mode == "star": + self.model.onItemStarToggle(item) + else: + self.model.onItemTrashToggle(item) + if self.undoStack is not None: + self.undoStack.push(cmd) + @QtCore.Slot() def _starCurrentItem(self) -> None: - self._toggleCurrentItem(self.view.itemStarToggle) + self._toggleStarTrash("star") @QtCore.Slot() def _trashCurrentItem(self) -> None: - self._toggleCurrentItem(self.view.itemTrashToggle) + self._toggleStarTrash("trash") @QtCore.Slot() def _fitCurrentColumn(self) -> None: diff --git a/src/instrumentserver/gui/instruments.py b/src/instrumentserver/gui/instruments.py index 66be25f..6bcbe1b 100644 --- a/src/instrumentserver/gui/instruments.py +++ b/src/instrumentserver/gui/instruments.py @@ -20,6 +20,7 @@ ItemBase, ) from .parameters import AnyInput, AnyInputForMethod, ParameterWidget +from .undo_commands import DeleteParameterCommand, ToggleEvalCommand # TODO: all styles set through a global style sheet. # TODO: [maybe] add a column for information on valid input values? @@ -213,14 +214,17 @@ def __init__( super().__init__(*args, **kwargs) self.fun = fun - - # Only used for logging purposes. self.fullName = fullName + self.undoStack: Any = None + self._suppress_eval_push: bool = False + self._full_name: Any = None + self._delegate: Any = None self.anyInput = AnyInputForMethod() self.anyInput.input.setPlaceholderText(str(inspect.signature(fun))) self.anyInput.input.setToolTip(self.getTooltipFromFun(fun)) self.anyInput.input.returnPressed.connect(self.runFun) + self.anyInput.doEval.toggled.connect(self._onDoEvalToggled) self.runButton = QtWidgets.QPushButton("Run", parent=self) self.runButton.clicked.connect(self.runFun) @@ -255,6 +259,11 @@ def runFun(self) -> None: self.runFailed.emit(str(e)) logger.warning(f"'{self.fullName}' Raised the following execution: {e}") + @QtCore.Slot(bool) + def _onDoEvalToggled(self, _: bool) -> None: + if not self._suppress_eval_push and self.undoStack is not None: + self.undoStack.push(ToggleEvalCommand(self._full_name, self._delegate)) + @classmethod def getTooltipFromFun(cls, fun: Callable) -> str: """ @@ -274,6 +283,7 @@ def __init__(self, unit: str = "", **kwargs: Any) -> None: self.unit = unit + class ParameterDelegate(DelegateBase): """ The delegate for the InstrumentParameters widget. @@ -286,6 +296,7 @@ def __init__(self, parent: Optional[QtCore.QObject] = None) -> None: # used to keep a reference to the widget. self.parameters: Dict[str, QtWidgets.QWidget] = {} self.navFilter: Optional["ValueCellNavigationFilter"] = None + self.undoStack: Optional[QtWidgets.QUndoStack] = None def createEditor( # type: ignore[override] self, @@ -301,6 +312,9 @@ def createEditor( # type: ignore[override] ret = ParameterWidget(element, widget) self.parameters[item.name] = ret # type: ignore[attr-defined] + ret.undoStack = self.undoStack + ret._full_name = item.name # type: ignore[attr-defined] + ret._delegate = self if self.navFilter is not None: if isinstance(ret.paramWidget, AnyInput): @@ -332,7 +346,7 @@ def __init__(self, treeView: InstrumentTreeViewBase) -> None: def registerWidget(self, widget: QtCore.QObject, index: QtCore.QModelIndex) -> None: self._widgetIndex[widget] = QtCore.QPersistentModelIndex(index) - def eventFilter(self, obj: QtCore.QObject, event: QtCore.QEvent) -> bool: + def eventFilter(self, obj: QtCore.QObject, event: QtCore.QEvent) -> bool: # type: ignore[override] if event.type() == QtCore.QEvent.Type.FocusIn: if obj in self._widgetIndex: idx = self._widgetIndex[obj] @@ -434,10 +448,20 @@ def updateParameter(self, bp: ParameterBroadcastBluePrint) -> None: if fullName not in self.instrument.list(): self.instrument.update() if fullName in self.instrument.list(): - self.addItem( + existing = self.findItems( fullName, - element=nestedAttributeFromString(self.instrument, fullName), + cast( + "QtCore.Qt.MatchFlags", + QtCore.Qt.MatchFlag.MatchExactly + | QtCore.Qt.MatchFlag.MatchRecursive, + ), + 0, ) + if not existing: + self.addItem( + fullName, + element=nestedAttributeFromString(self.instrument, fullName), + ) elif bp.action == "parameter-deletion": self.removeItem(fullName) @@ -546,6 +570,7 @@ def __init__( modelKwargs["sub_port"] = kwargs.pop("sub_port") shortcutManager = kwargs.pop("shortcutManager", None) + undo_stack = kwargs.pop("undo_stack", None) super().__init__( instrument=instrument, @@ -559,10 +584,18 @@ def __init__( **modelKwargs, ) + self.undoStack: QtWidgets.QUndoStack = ( + undo_stack if undo_stack is not None else QtWidgets.QUndoStack(self) + ) + self.view.delegate.undoStack = self.undoStack + for widget in self.view.delegate.parameters.values(): + widget.undoStack = self.undoStack + self.shortcutManager.register("undo", self.undoStack.undo, self) + self.shortcutManager.register("redo", self.undoStack.redo, self) + self.view.editCurrentParameter.connect(self._focusToParameterValue) self.view.clearCurrentParameter.connect(self._clearCurrentParameter) - def connectSignals(self) -> None: super().connectSignals() self.model.itemNewValue.connect(self.view.onItemNewValue) @@ -586,13 +619,13 @@ def _refreshCurrentItem(self) -> None: @QtCore.Slot() def _togglePythonCurrentItem(self) -> None: - self._withCurrentParameter( - lambda w: ( - w.paramWidget.doEval.toggle() - if isinstance(w.paramWidget, AnyInput) - else None - ) - ) + item = self._getCurrentItem() + if item is None: + return + widget = self.view.delegate.parameters.get(item.name) + if widget is None or not isinstance(widget.paramWidget, AnyInput): + return + widget.paramWidget.doEval.toggle() @QtCore.Slot() def _focusToParameterValue(self) -> None: @@ -603,7 +636,7 @@ def _focusToParameterValue(self) -> None: else w.paramWidget.setFocus() ) ) - + @QtCore.Slot() def _clearCurrentParameter(self) -> None: self._withCurrentParameter( @@ -611,13 +644,12 @@ def _clearCurrentParameter(self) -> None: w.paramWidget.input.clear() if isinstance(w.paramWidget, AnyInput) else w.paramWidget.clear() - if hasattr(w.paramWidget, 'clear') + if hasattr(w.paramWidget, "clear") else None ) ) - # ----------------- Parameters Display Classes - Ending -------------------------------- # ----------------- Parameters Manager Classes - Beginning ----------------------------- @@ -640,6 +672,9 @@ def createEditor( # type: ignore[override] ret = ParameterWidget(parameter=element, parent=widget, additionalWidgets=[rw]) self.parameters[item.name] = ret # type: ignore[attr-defined] + ret.undoStack = self.undoStack + ret._full_name = item.name # type: ignore[attr-defined] + ret._delegate = self if self.navFilter is not None: if isinstance(ret.paramWidget, AnyInput): @@ -800,8 +835,25 @@ def refreshAll(self) -> None: self.profileManager.refresh() def removeParameter(self, fullName: str) -> None: - if self.instrument.has_param(fullName): - self.instrument.remove_parameter(fullName) + if not self.instrument.has_param(fullName): + return + param = nestedAttributeFromString(self.instrument, fullName) + try: + value = param.get() + except Exception: + value = None + unit = getattr(param, "unit", "") + items = self.model.findItems( + fullName, + QtCore.Qt.MatchFlag.MatchExactly | QtCore.Qt.MatchFlag.MatchRecursive, + 0, + ) + item = items[0] if items else None + star = item.star if item is not None else False + trash = item.trash if item is not None else False + self.undoStack.push( + DeleteParameterCommand(self, fullName, value, unit, star, trash) + ) def addParameter(self, fullName: str, value: Any, unit: str) -> None: try: @@ -875,6 +927,7 @@ def __init__(self, parent: Optional[QtCore.QObject] = None) -> None: self.methods: Dict[str, "MethodDisplay"] = {} self.navFilter: Optional[ValueCellNavigationFilter] = None + self.undoStack: Any = None def createEditor( # type: ignore[override] self, @@ -885,6 +938,9 @@ def createEditor( # type: ignore[override] item = self.getItem(index) element = item.element # type: ignore[attr-defined] ret = MethodDisplay(element, item.name, parent=widget) # type: ignore[attr-defined] + ret.undoStack = self.undoStack + ret._full_name = item.name # type: ignore[attr-defined] + ret._delegate = self parent = self.parent() assert hasattr(parent, "clearAlertsAction") @@ -934,6 +990,7 @@ def __init__(self, instrument: Any, **kwargs: Any) -> None: modelKwargs["itemsHide"] = kwargs.pop("methods-hide") shortcutManager = kwargs.pop("shortcutManager", None) + undo_stack = kwargs.pop("undo_stack", None) super().__init__( instrument=instrument, @@ -944,6 +1001,15 @@ def __init__(self, instrument: Any, **kwargs: Any) -> None: **modelKwargs, ) + self.undoStack: QtWidgets.QUndoStack = ( + undo_stack if undo_stack is not None else QtWidgets.QUndoStack(self) + ) + self.shortcutManager.register("undo", self.undoStack.undo, self) + self.shortcutManager.register("redo", self.undoStack.redo, self) + self.view.delegate.undoStack = self.undoStack + for widget in self.view.delegate.methods.values(): + widget.undoStack = self.undoStack + self.view.editCurrentParameter.connect(self._focusToMethodValue) self.view.clearCurrentParameter.connect(self._clearCurrentMethod) @@ -963,7 +1029,13 @@ def _withCurrentMethod(self, callback: Callable[["MethodDisplay"], None]) -> Non @QtCore.Slot() def _togglePythonCurrentItem(self) -> None: - self._withCurrentMethod(lambda w: w.anyInput.doEval.toggle()) + item = self._getCurrentItem() + if item is None: + return + widget = self.view.delegate.methods.get(item.name) + if widget is None: + return + widget.anyInput.doEval.toggle() @QtCore.Slot() def _focusToMethodValue(self) -> None: @@ -1017,7 +1089,9 @@ def __init__( self.splitter.setOrientation(QtCore.Qt.Orientation.Vertical) self.parametersList = InstrumentParameters(instrument=ins, **modelKwargs) - self.methodsList = InstrumentMethods(instrument=ins, **modelKwargs) + self.methodsList = InstrumentMethods( + instrument=ins, undo_stack=self.parametersList.undoStack, **modelKwargs + ) self.instrumentNameLabel = QtWidgets.QLabel(ins_label) self._layout.addWidget(self.instrumentNameLabel) diff --git a/src/instrumentserver/gui/parameters.py b/src/instrumentserver/gui/parameters.py index c1d7f15..d8c9ac4 100644 --- a/src/instrumentserver/gui/parameters.py +++ b/src/instrumentserver/gui/parameters.py @@ -9,6 +9,7 @@ from ..params import ParameterTypes, paramTypeFromVals from . import keepSmallHorizontally from .misc import AlertLabel +from .undo_commands import SetParameterCommand, ToggleEvalCommand logger = logging.getLogger(__name__) @@ -63,6 +64,11 @@ def __init__( self._parameter = parameter self._getMethod: Callable[[], Optional[Any]] = lambda: None self._setMethod = lambda x: None + self.undoStack: Any = None + self._suppress_command_push: bool = False + self._suppress_eval_push: bool = False + self._full_name: Any = None + self._delegate: Any = None layout = QtWidgets.QGridLayout(self) self.getButton = QtWidgets.QPushButton( @@ -141,6 +147,7 @@ def __init__( self.paramWidget.setValue(parameter()) self.paramWidget.inputChanged.connect(self.setPending) self.paramWidget.input.returnPressed.connect(self.onReturnPressed) + self.paramWidget.doEval.toggled.connect(self._onDoEvalToggled) self._getMethod = self.paramWidget.value self._setMethod = self.paramWidget.setValue @@ -184,14 +191,33 @@ def onReturnPressed(self) -> None: self.setButton.setFocus() def setParameter(self, value: Any) -> None: + if not self._suppress_command_push and self.undoStack is not None: + try: + old_value = self._parameter.get() + except Exception: + old_value = None + try: self._parameter.set(value) + actual = self._parameter.get() + if actual != value: + self.parameterSetError.emit( + f"Parameter rejected value {value!r} (current: {actual!r})" + ) + return except Exception as e: self.parameterSetError.emit( f"Could not set parameter, raised {type(e)}: {e.args}" ) return + if not self._suppress_command_push and self.undoStack is not None: + self.undoStack.push( + SetParameterCommand( + self._parameter, self._full_name, self._delegate, old_value, value + ) + ) + self.parameterSet.emit(value) def setPending(self, value: Any) -> None: @@ -207,6 +233,11 @@ def setWidgetFromParameter(self) -> None: self._setMethod(val) self.parameterSet.emit(val) + @QtCore.Slot(bool) + def _onDoEvalToggled(self, _: bool) -> None: + if not self._suppress_eval_push and self.undoStack is not None: + self.undoStack.push(ToggleEvalCommand(self._full_name, self._delegate)) + class AnyInput(QtWidgets.QWidget): #: Signal(str) -- diff --git a/src/instrumentserver/gui/shortcuts.py b/src/instrumentserver/gui/shortcuts.py index 77c8096..b697e7e 100644 --- a/src/instrumentserver/gui/shortcuts.py +++ b/src/instrumentserver/gui/shortcuts.py @@ -25,6 +25,8 @@ class KeyboardShortcutManager: REGISTRY: dict[str, tuple[str, str]] = { # action_id: (default_key_sequence, description) + "undo": ("Ctrl+Z", "Undoes the previous action"), + "redo": ("Ctrl+Shift+Z", "Redoes the previous action"), "jump_filter": ("Ctrl+F", "Jump cursor to the filter search bar"), "collapse_all": ("Ctrl+Shift+E", "Collapse all tree nodes"), "expand_all": ("Ctrl+E", "Expand all tree nodes"), diff --git a/src/instrumentserver/gui/undo_commands.py b/src/instrumentserver/gui/undo_commands.py new file mode 100644 index 0000000..3eca9d5 --- /dev/null +++ b/src/instrumentserver/gui/undo_commands.py @@ -0,0 +1,200 @@ +from typing import Any, Tuple + +from instrumentserver import QtCore, QtGui, QtWidgets +from instrumentserver.helpers import nestedAttributeFromString + + +class SetParameterCommand(QtWidgets.QUndoCommand): + def __init__( + self, param: Any, name: str, delegate: Any, old_val: Any, new_val: Any + ) -> None: + super().__init__(f"Set {param.name}") + self._param = param + self._name = name + self._delegate = delegate + self._old = old_val + self._new = new_val + self._first = True + + def _get_widget(self) -> Any: + if hasattr(self._delegate, "parameters"): + return self._delegate.parameters.get(self._name) + return None + + def _apply(self, value: Any) -> None: + widget = self._get_widget() + if widget is None: + return + widget._suppress_command_push = True + try: + self._param.set(value) + actual = self._param.get() + if actual != value: + widget.parameterSetError.emit( + f"Could not set parameter: value is {actual!r}, not {value!r}" + ) + widget._setMethod(actual) + else: + widget._setMethod(value) + widget.parameterSet.emit(value) + except Exception as e: + widget.parameterSetError.emit( + f"Could not set parameter, raised {type(e)}: {e.args}" + ) + finally: + widget._suppress_command_push = False + + def undo(self) -> None: + self._apply(self._old) + + def redo(self) -> None: + if self._first: + self._first = False + return + self._apply(self._new) + + +def _restore_star_trash(item: Any, star: bool, trash: bool) -> None: + item.star = star + item.trash = trash + if star: + item.setIcon(QtGui.QIcon(":/icons/star.svg")) + elif trash: + item.setIcon(QtGui.QIcon(":/icons/trash.svg")) + else: + item.setIcon(QtGui.QIcon()) + + +class ToggleStarTrashCommand(QtWidgets.QUndoCommand): + def __init__(self, item: Any, model: Any, mode: str) -> None: + super().__init__(f"Toggle {mode} {item.name}") + self._name = item.name + self._model = model + self._mode = mode + self._old_star = item.star + self._old_trash = item.trash + self._first = True + + def _get_item(self) -> Any: + items = self._model.findItems( + self._name, + QtCore.Qt.MatchFlag.MatchExactly | QtCore.Qt.MatchFlag.MatchRecursive, + 0, + ) + return items[0] if items else None + + def undo(self) -> None: + item = self._get_item() + if item is not None: + _restore_star_trash(item, self._old_star, self._old_trash) + + def redo(self) -> None: + if self._first: + self._first = False + return + item = self._get_item() + if item is not None: + if self._mode == "star": + self._model.onItemStarToggle(item) + else: + self._model.onItemTrashToggle(item) + + +class ToggleEvalCommand(QtWidgets.QUndoCommand): + def __init__(self, name: str, delegate: Any) -> None: + super().__init__("Toggle eval") + self._name = name + self._delegate = delegate + self._first = True + + def _get_owner_and_eval(self) -> Tuple[Any, Any]: + if hasattr(self._delegate, "parameters"): + owner = self._delegate.parameters.get(self._name) + if ( + owner is not None + and hasattr(owner, "paramWidget") + and hasattr(owner.paramWidget, "doEval") + ): + return owner, owner.paramWidget.doEval + if hasattr(self._delegate, "methods"): + owner = self._delegate.methods.get(self._name) + if owner is not None and hasattr(owner, "anyInput"): + return owner, owner.anyInput.doEval + return None, None + + def _apply(self) -> None: + owner, eval_widget = self._get_owner_and_eval() + if owner is None or eval_widget is None: + return + owner._suppress_eval_push = True + try: + eval_widget.toggle() + finally: + owner._suppress_eval_push = False + + def undo(self) -> None: + self._apply() + + def redo(self) -> None: + if self._first: + self._first = False + return + self._apply() + + +class DeleteParameterCommand(QtWidgets.QUndoCommand): + """Undo/redo a parameter deletion in ParameterManagerGui.""" + + def __init__( + self, + parent_gui: Any, + full_name: str, + value: Any, + unit: str, + star: bool, + trash: bool, + ) -> None: + super().__init__(f"Delete {full_name}") + self._gui = parent_gui + self._full_name = full_name + self._value = value + self._unit = unit + self._star = star + self._trash = trash + widget = parent_gui.view.delegate.parameters.get(full_name) + if ( + widget is not None + and hasattr(widget, "paramWidget") + and hasattr(widget.paramWidget, "doEval") + ): + self._doEval: bool = widget.paramWidget.doEval.isChecked() + else: + self._doEval = True + + def undo(self) -> None: + self._gui.instrument.add_parameter( + self._full_name, initial_value=self._value, unit=self._unit + ) + param = nestedAttributeFromString(self._gui.instrument, self._full_name) + item = self._gui.model.addItem( + fullName=self._full_name, + star=False, + trash=False, + element=param, + unit=self._unit, + ) + _restore_star_trash(item, self._star, self._trash) + widget = self._gui.view.delegate.parameters.get(self._full_name) + if ( + widget is not None + and hasattr(widget, "paramWidget") + and hasattr(widget.paramWidget, "doEval") + ): + widget._suppress_eval_push = True + widget.paramWidget.doEval.setChecked(self._doEval) + widget._suppress_eval_push = False + + def redo(self) -> None: + if self._gui.instrument.has_param(self._full_name): + self._gui.instrument.remove_parameter(self._full_name) + self._gui.model.removeItem(self._full_name) From adc3e5d86cb09dd4de870737676eff257b4717db Mon Sep 17 00:00:00 2001 From: olivers3uiuc Date: Tue, 30 Jun 2026 13:53:22 -0400 Subject: [PATCH 3/3] small fixes --- src/instrumentserver/gui/base_instrument.py | 8 +++----- src/instrumentserver/gui/undo_commands.py | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/instrumentserver/gui/base_instrument.py b/src/instrumentserver/gui/base_instrument.py index 561c6bd..b7e7d0d 100644 --- a/src/instrumentserver/gui/base_instrument.py +++ b/src/instrumentserver/gui/base_instrument.py @@ -108,8 +108,7 @@ from instrumentserver import QtCore, QtGui, QtWidgets from instrumentserver.gui.shortcuts import KeyboardShortcutManager - -from .undo_commands import ToggleStarTrashCommand +from instrumentserver.gui.undo_commands import ToggleStarTrashCommand class ItemBase(QtGui.QStandardItem): @@ -967,15 +966,14 @@ def _toggleStarTrash(self, mode: str, item: Optional["ItemBase"] = None) -> None item = self._getCurrentItem() if item is None: return + self.view.lastSelectedItem = item if self.undoStack is not None: cmd = ToggleStarTrashCommand(item, self.model, mode) - self.view.lastSelectedItem = item + self.undoStack.push(cmd) if mode == "star": self.model.onItemStarToggle(item) else: self.model.onItemTrashToggle(item) - if self.undoStack is not None: - self.undoStack.push(cmd) @QtCore.Slot() def _starCurrentItem(self) -> None: diff --git a/src/instrumentserver/gui/undo_commands.py b/src/instrumentserver/gui/undo_commands.py index 3eca9d5..330b49b 100644 --- a/src/instrumentserver/gui/undo_commands.py +++ b/src/instrumentserver/gui/undo_commands.py @@ -197,4 +197,4 @@ def undo(self) -> None: def redo(self) -> None: if self._gui.instrument.has_param(self._full_name): self._gui.instrument.remove_parameter(self._full_name) - self._gui.model.removeItem(self._full_name) + self._gui.model.removeItem(self._full_name)