[work-ctme] Make theme delete undoable with Ctrl+Z #59

Merged
keeper merged 2 commits from work-ctme-theme-delete-is-not-undoable-ctrl-z into main 2026-10-08 15:29:27 +00:00
Collaborator

Adds a delete-undo layer over the per-theme history. Ctrl+Z right after a delete restores the theme at its old index with its own history. Ctrl+Shift+Z / Ctrl+Y delete it again. Any other action drops the delete stacks. Adds studio/tests/delete-undo.test.js.

🤖 Generated with Claude Code

https://claude.ai/code/session_019QVQwoVfUVoPLD3SJ9jcpe

Adds a delete-undo layer over the per-theme history. Ctrl+Z right after a delete restores the theme at its old index with its own history. Ctrl+Shift+Z / Ctrl+Y delete it again. Any other action drops the delete stacks. Adds studio/tests/delete-undo.test.js. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_019QVQwoVfUVoPLD3SJ9jcpe
[work-ctme] Make theme delete undoable with Ctrl+Z
All checks were successful
Mossfire web app / test (pull_request) Successful in 39s
Mossfire web app / publish (tema.uhyre.dk, global) (pull_request) Has been skipped
Mossfire web app / publish (theme.home.dpis.dk, home) (pull_request) Has been skipped
b206f2a82b
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QVQwoVfUVoPLD3SJ9jcpe
keeper requested changes 2026-10-08 15:12:50 +00:00
Dismissed
keeper left a comment

Keeper review of PR #59 (head b206f2a).

Routine: one IIFE in studio/js/app.js plus a test and a README line. No data loss, security/auth/secrets/network/lockout surface. PR labels and requested reviewers are empty; no review:human on the PR or bead. Head CI is success and mergeable=true.

The delete-undo feature is correct and well built. I read the diff and ran the tests in a clean checkout of the head:

  • All studio node tests plus build_data.test.py pass.
  • Traced the trash/redoTrash layering: record, goHistory, select (without keepTrash) drop the stacks; deleteTheme/undoDelete/redoDelete push/pop and re-enable #undo/#redo; index restore and replacement-Mossfire removal work. Wrote extra scenarios (delete non-current with no history, undo current theme then its own edit, redo after undo) and they pass too.

Blocking finding work-stjr: the new test never runs in CI. This PR adds studio/tests/delete-undo.test.js and lists it in studio/README.md, but .forgejo/workflows/pages.yaml lists every studio test by hand and was not updated. Every earlier test-adding PR (#55 windows-terminal, #56 rofi) added the matching line to that workflow in the same commit; this one does not, so the CI Tests step skips it. Add node studio/tests/delete-undo.test.js to the Tests step.

Requesting changes.

Keeper review of PR #59 (head b206f2a). Routine: one IIFE in `studio/js/app.js` plus a test and a README line. No data loss, security/auth/secrets/network/lockout surface. PR labels and requested reviewers are empty; no `review:human` on the PR or bead. Head CI is success and mergeable=true. The delete-undo feature is correct and well built. I read the diff and ran the tests in a clean checkout of the head: - All studio node tests plus `build_data.test.py` pass. - Traced the trash/redoTrash layering: `record`, `goHistory`, `select` (without `keepTrash`) drop the stacks; `deleteTheme`/`undoDelete`/`redoDelete` push/pop and re-enable `#undo`/`#redo`; index restore and replacement-Mossfire removal work. Wrote extra scenarios (delete non-current with no history, undo current theme then its own edit, redo after undo) and they pass too. Blocking finding work-stjr: the new test never runs in CI. This PR adds `studio/tests/delete-undo.test.js` and lists it in `studio/README.md`, but `.forgejo/workflows/pages.yaml` lists every studio test by hand and was not updated. Every earlier test-adding PR (#55 windows-terminal, #56 rofi) added the matching line to that workflow in the same commit; this one does not, so the CI `Tests` step skips it. Add `node studio/tests/delete-undo.test.js` to the `Tests` step. Requesting changes.
[work-ctme] Run delete-undo test in CI
All checks were successful
Mossfire web app / test (pull_request) Successful in 39s
Mossfire web app / publish (tema.uhyre.dk, global) (pull_request) Has been skipped
Mossfire web app / publish (theme.home.dpis.dk, home) (pull_request) Has been skipped
8b0377bd24
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QVQwoVfUVoPLD3SJ9jcpe
keeper left a comment

Keeper re-review of PR #59 (head 8b0377b).

Routine: one IIFE in studio/js/app.js, one test file, a README line and one CI line. No data loss, security/auth/secrets/network/lockout surface. No review:human on the PR or on work-ctme, no human requested reviewer, no AGENTS.md/CLAUDE.md in the repo. Head CI is success and mergeable=true.

The earlier request-changes (finding work-stjr: the new test never ran in CI) is fixed by 8b0377b, which adds node studio/tests/delete-undo.test.js to the Tests step in .forgejo/workflows/pages.yaml, matching #55/#56. work-stjr is closed.

Checked on a clean checkout of the head:

  • Diff vs main is exactly the four files above; no stray edits.
  • Ran the full CI test list (build_data.test.py plus all studio node tests) — all pass, including delete-undo.
  • Traced the trash/redoTrash layer against acceptance criteria 1-9: index restore, own-history restore, replacement-Mossfire removal, two-delete LIFO order, redo round trip, dropTrash on record/goHistory/select/addTheme/import/preset/generate-submit and on history-menu steps, #undo/#redo enabling, and the typing/generator shortcut guards. Index math and the null-replacement paths are correct.
  • node --check on both changed JS files.

Nits, not blocking, no beads needed: the entry's wasCurrent field is stored but never read (the bead's decision is that a restored theme always becomes current); opening then cancelling the generator or wand without committing does not drop the stacks, so Ctrl+Z still restores a just-deleted theme — acceptable since no theme changed.

Approving and merging.

Keeper re-review of PR #59 (head 8b0377b). Routine: one IIFE in `studio/js/app.js`, one test file, a README line and one CI line. No data loss, security/auth/secrets/network/lockout surface. No `review:human` on the PR or on work-ctme, no human requested reviewer, no AGENTS.md/CLAUDE.md in the repo. Head CI is success and mergeable=true. The earlier request-changes (finding work-stjr: the new test never ran in CI) is fixed by 8b0377b, which adds `node studio/tests/delete-undo.test.js` to the `Tests` step in `.forgejo/workflows/pages.yaml`, matching #55/#56. work-stjr is closed. Checked on a clean checkout of the head: - Diff vs main is exactly the four files above; no stray edits. - Ran the full CI test list (build_data.test.py plus all studio node tests) — all pass, including delete-undo. - Traced the trash/redoTrash layer against acceptance criteria 1-9: index restore, own-history restore, replacement-Mossfire removal, two-delete LIFO order, redo round trip, dropTrash on record/goHistory/select/addTheme/import/preset/generate-submit and on history-menu steps, `#undo`/`#redo` enabling, and the typing/generator shortcut guards. Index math and the null-replacement paths are correct. - `node --check` on both changed JS files. Nits, not blocking, no beads needed: the entry's `wasCurrent` field is stored but never read (the bead's decision is that a restored theme always becomes current); opening then cancelling the generator or wand without committing does not drop the stacks, so Ctrl+Z still restores a just-deleted theme — acceptable since no theme changed. Approving and merging.
keeper left a comment

Keeper re-review of PR #59 at 8b0377b. Routine; tests pass; previous CI finding work-stjr fixed.

Keeper re-review of PR #59 at 8b0377b. Routine; tests pass; previous CI finding work-stjr fixed.
keeper approved these changes 2026-10-08 15:29:19 +00:00
keeper left a comment

Keeper re-review of PR #59 (head 8b0377b).

Routine: one IIFE in studio/js/app.js, one test file, a README line and one CI line. No data loss, security/auth/secrets/network/lockout surface. No review:human on the PR or on work-ctme, no human requested reviewer, no AGENTS.md/CLAUDE.md in the repo. Head CI is success and mergeable=true.

The earlier request-changes (finding work-stjr: the new test never ran in CI) is fixed by 8b0377b, which adds node studio/tests/delete-undo.test.js to the Tests step in .forgejo/workflows/pages.yaml, matching #55/#56. work-stjr is closed.

Checked on a clean checkout of the head:

  • Diff vs main is exactly the four files above; no stray edits.
  • Ran the full CI test list (build_data.test.py plus all studio node tests) — all pass, including delete-undo.
  • Traced the trash/redoTrash layer against acceptance criteria 1-9: index restore, own-history restore, replacement-Mossfire removal, two-delete LIFO order, redo round trip, dropTrash on record/goHistory/select/addTheme/import/preset/generate-submit and on history-menu steps, #undo/#redo enabling, and the typing/generator shortcut guards. Index math and the null-replacement paths are correct.
  • node --check on both changed JS files.

Nits, not blocking, no beads needed: the entry's wasCurrent field is stored but never read (the bead's decision is that a restored theme always becomes current); opening then cancelling the generator or wand without committing does not drop the stacks, so Ctrl+Z still restores a just-deleted theme — acceptable since no theme changed.

Approving and merging.

Keeper re-review of PR #59 (head 8b0377b). Routine: one IIFE in `studio/js/app.js`, one test file, a README line and one CI line. No data loss, security/auth/secrets/network/lockout surface. No `review:human` on the PR or on work-ctme, no human requested reviewer, no AGENTS.md/CLAUDE.md in the repo. Head CI is success and mergeable=true. The earlier request-changes (finding work-stjr: the new test never ran in CI) is fixed by 8b0377b, which adds `node studio/tests/delete-undo.test.js` to the `Tests` step in `.forgejo/workflows/pages.yaml`, matching #55/#56. work-stjr is closed. Checked on a clean checkout of the head: - Diff vs main is exactly the four files above; no stray edits. - Ran the full CI test list (build_data.test.py plus all studio node tests) — all pass, including delete-undo. - Traced the trash/redoTrash layer against acceptance criteria 1-9: index restore, own-history restore, replacement-Mossfire removal, two-delete LIFO order, redo round trip, dropTrash on record/goHistory/select/addTheme/import/preset/generate-submit and on history-menu steps, `#undo`/`#redo` enabling, and the typing/generator shortcut guards. Index math and the null-replacement paths are correct. - `node --check` on both changed JS files. Nits, not blocking, no beads needed: the entry's `wasCurrent` field is stored but never read (the bead's decision is that a restored theme always becomes current); opening then cancelling the generator or wand without committing does not drop the stacks, so Ctrl+Z still restores a just-deleted theme — acceptable since no theme changed. Approving and merging.
keeper merged commit fddd9a7fa6 into main 2026-10-08 15:29:27 +00:00
keeper deleted branch work-ctme-theme-delete-is-not-undoable-ctrl-z 2026-10-08 15:29:27 +00:00
Sign in to join this conversation.
No reviewers
No labels
review:human
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
daniel/mossfire!59
No description provided.