diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d443c8c..3706a8d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,7 +21,7 @@ jobs: # everywhere or only on Linux?" is the first question worth answering. fail-fast: false matrix: - os: [ubuntu-latest, windows-latest] + os: [ubuntu-latest, windows-latest, macos-latest] steps: - uses: actions/checkout@v4 diff --git a/SPEC.md b/SPEC.md index 22d8b92..c672203 100644 --- a/SPEC.md +++ b/SPEC.md @@ -832,6 +832,28 @@ Implementation notes: - Verified end to end against a real shell: two firings, one tab named after the job, both runs landing in the same terminal. +### Phase 4t — Reordering presets ✅ done +Move Up / Move Down in every Manage dialog. The order in that list is the +order they appear in their menu, and for Commands in the sidebar too, so it +was the one property of a preset you could not set. + +- [x] `PresetStore.swap()` rather than a move: one list holds all three + categories interleaved, and the two entries being exchanged are + neighbours within their category but rarely adjacent in the file. + Swapping their positions leaves every other preset exactly where it + was - there is a test asserting Commands do not shuffle when a Macro + moves past another. +- [x] Offered for all three categories, not only Macros. It is the same + dialog and the same code, and "why can I order Macros but not + Commands?" is a worse answer than the feature. +- [x] The selection follows the entry that moved, so pressing Move Up twice + moves one preset two places rather than moving two presets - the same + rule the right-click menu order editor uses. +- [x] Known limit, documented in USAGE.md rather than fixed: the menus nest + grouped entries under their group, so reordering across a group + boundary changes the stored order without changing what the menu + shows. + ## Open Questions / Deferred ### Start-up latency — measured, not yet decided diff --git a/src/qtxterm/assets/USAGE.md b/src/qtxterm/assets/USAGE.md index bff7380..be91dc1 100644 --- a/src/qtxterm/assets/USAGE.md +++ b/src/qtxterm/assets/USAGE.md @@ -200,6 +200,17 @@ Two things to know: - **A job names its Macro.** Rename or delete that Macro and the job says so in the status bar rather than running something else. +## Ordering + +Each Manage dialog has **Move Up** and **Move Down** under its list. The +order there is the order they appear in — the Macros menu, the right-click +Command submenu, and the sidebar buttons. + +One thing to know: entries with a **Group** are shown nested under that group +in the menu, so moving one only reorders it *within* its group. Moving a +grouped entry past an ungrouped one changes the stored order without visibly +changing the menu. + ## Creating and editing Each menu manages its own category: **Manage Commands...** under Commands, diff --git a/src/qtxterm/preset_editor.py b/src/qtxterm/preset_editor.py index 37e8acb..2d90939 100644 --- a/src/qtxterm/preset_editor.py +++ b/src/qtxterm/preset_editor.py @@ -139,6 +139,22 @@ def __init__( button_row.addWidget(new_button) button_row.addWidget(delete_button) left.addLayout(button_row) + + # Order here is the order they appear in their menu (and, for + # Commands, in the sidebar), so it is worth being able to set. + order_row = QHBoxLayout() + up_button = QPushButton("Move Up") + down_button = QPushButton("Move Down") + for button, offset in ((up_button, -1), (down_button, 1)): + button.setToolTip( + "Reorder within this list. Grouped entries are shown under " + "their group in the menu, so order applies within a group." + ) + button.clicked.connect( + lambda _checked=False, delta=offset: self._move_current(delta) + ) + order_row.addWidget(button) + left.addLayout(order_row) if self._is_selection: # Defaults are only seeded on first run, so an install that # predates Selection Actions has no other way to get the worked @@ -327,6 +343,24 @@ def _add_examples(self) -> None: self._store.add(preset) self._reload_list() + def _move_current(self, offset: int) -> None: + """Swap the selected preset with its neighbour in this category. + + Neighbour in the *list*, not in the file: the other categories live + in the same list and must not shuffle because a Macro moved. + """ + indexed = self._indexed_presets() + row = self._list.currentRow() + target_row = row + offset + if row < 0 or not 0 <= target_row < len(indexed): + return + + self._store.swap(indexed[row][0], indexed[target_row][0]) + self._reload_list() + # Follow the entry that moved, so pressing Move Up twice moves one + # preset two places rather than moving two presets. + self._list.setCurrentRow(target_row) + def _delete_preset(self) -> None: if self._current_index is None: return diff --git a/src/qtxterm/presets.py b/src/qtxterm/presets.py index d196b8e..566e22e 100644 --- a/src/qtxterm/presets.py +++ b/src/qtxterm/presets.py @@ -226,3 +226,17 @@ def update(self, index: int, preset: Preset) -> None: def delete(self, index: int) -> None: del self.presets[index] self.save() + + def swap(self, first: int, second: int) -> None: + """Exchange two presets, which is how reordering is expressed. + + A swap rather than a move: one list holds all three categories + interleaved, and the two entries being exchanged are neighbours + within their own category but rarely adjacent in the file. Swapping + their positions leaves every other preset exactly where it was. + """ + self.presets[first], self.presets[second] = ( + self.presets[second], + self.presets[first], + ) + self.save() diff --git a/tests/test_preset_editor.py b/tests/test_preset_editor.py index 40566a1..9f9e017 100644 --- a/tests/test_preset_editor.py +++ b/tests/test_preset_editor.py @@ -13,6 +13,8 @@ from qtxterm import preset_editor from qtxterm.preset_editor import PresetEditorDialog +from qtxterm.preset_menu import MacrosMenu +from qtxterm.terminal_tabs import TerminalTabWidget from qtxterm.presets import ( STEP_DOWN, STEP_RIGHT, @@ -27,6 +29,7 @@ SELECTION_PLACEHOLDER, Preset, PresetStore, + category_of, ) @@ -355,3 +358,95 @@ def test_saving_keeps_the_separator_lines(qtbot, tmp_path: Path) -> None: dialog._save_current() assert dialog._store.presets[0].lines == ["first", "--- down", "second"] + + +def macro_names(store: PresetStore) -> list[str]: + return [p.name for p in store.presets if category_of(p) == CATEGORY_MACROS] + + +def test_moving_a_macro_down_reorders_it(qtbot, tmp_path: Path) -> None: + store = make_store(tmp_path) + store.presets = [ + Preset(name="First", lines=["a"], target="new_tab"), + Preset(name="Second", lines=["b"], target="new_tab"), + Preset(name="Third", lines=["c"], target="new_tab"), + ] + store.save() + dialog = PresetEditorDialog(store, category=CATEGORY_MACROS) + qtbot.addWidget(dialog) + + dialog._list.setCurrentRow(0) + dialog._move_current(1) + + assert macro_names(store) == ["Second", "First", "Third"] + # The selection follows the entry that moved, so pressing it again moves + # the same one further. + dialog._move_current(1) + assert macro_names(store) == ["Second", "Third", "First"] + + +def test_moving_past_either_end_does_nothing(qtbot, tmp_path: Path) -> None: + store = make_store(tmp_path) + store.presets = [ + Preset(name="First", lines=["a"], target="new_tab"), + Preset(name="Second", lines=["b"], target="new_tab"), + ] + store.save() + dialog = PresetEditorDialog(store, category=CATEGORY_MACROS) + qtbot.addWidget(dialog) + + dialog._list.setCurrentRow(0) + dialog._move_current(-1) + dialog._list.setCurrentRow(1) + dialog._move_current(1) + + assert macro_names(store) == ["First", "Second"] + + +def test_reordering_macros_leaves_the_other_categories_alone( + qtbot, tmp_path: Path +) -> None: + """One list holds all three categories interleaved, so a Macro moving must + not shuffle the Commands sitting between them.""" + store = make_store(tmp_path) + store.presets = [ + Preset(name="MacroA", lines=["a"], target="new_tab"), + Preset(name="CommandX", lines=["x"], target="active"), + Preset(name="MacroB", lines=["b"], target="new_tab"), + Preset(name="CommandY", lines=["y"], target="active"), + ] + store.save() + dialog = PresetEditorDialog(store, category=CATEGORY_MACROS) + qtbot.addWidget(dialog) + + dialog._list.setCurrentRow(0) + dialog._move_current(1) + + assert macro_names(store) == ["MacroB", "MacroA"] + commands = [p.name for p in store.presets if category_of(p) == CATEGORY_COMMANDS] + assert commands == ["CommandX", "CommandY"] + + +def test_the_new_order_is_saved_and_shown_in_the_menu(qtbot, tmp_path: Path) -> None: + store = make_store(tmp_path) + store.presets = [ + Preset(name="First", lines=["a"], target="new_tab"), + Preset(name="Second", lines=["b"], target="new_tab"), + ] + store.save() + dialog = PresetEditorDialog(store, category=CATEGORY_MACROS) + qtbot.addWidget(dialog) + + dialog._list.setCurrentRow(1) + dialog._move_current(-1) + + assert macro_names(PresetStore(path=tmp_path / "presets.json")) == [ + "Second", + "First", + ] + + tabs = TerminalTabWidget() + qtbot.addWidget(tabs) + menu = MacrosMenu(store, tabs) + listed = [a.text() for a in menu.actions()][:2] + assert listed == ["Second", "First"]