fix(em): em move was broken in production; add em undo + recent-folders (Phase 3)#497
Merged
Conversation
…dd em undo + recent-folders (Phase 3) em move <ID> <folder> was broken in production: _em_move and its adapter _em_hml_move were each defined twice in email-dispatcher.zsh/em-himalaya.zsh, and zsh's last-definition-wins semantics meant the two LIVE definitions were incompatible with each other — the live _em_move called _em_hml_move with the numeric message ID in the source-folder position. Every move would attempt to move a message from a folder named after its own ID. Same failure class as the flag/star case-duplicate bug fixed earlier this session (PR #493), this time a duplicate function definition rather than a duplicate case label. Caught by tests/test-em-move-restore.zsh, which was never wired into tests/run-all.sh (orphaned, like test-em-ai-switch.zsh was) — it now fails 3/8 -> passes 8/8 against the fixed source and is registered. Kept the multi-ID interface (em move <FOLDER> <ID>...) since it matches em move --help, MASTER-DISPATCHER-GUIDE.md, and the orphaned test file. Corrected the same stale <ID>-first interface across QUICK-REFERENCE.md, MASTER-DISPATCHER-GUIDE.md, REFCARD-EMAIL-DISPATCHER.md (which had both a stale and a correct em move row), EMAIL-DISPATCHER-GUIDE.md, EMAIL-COOKBOOK.md, and man/man1/em.1 — plus a leftover "em flag = alias for em star" line the Phase 1+2 docs sweep missed. Phase 3 (docs/specs/SPEC-em-ux-refactor-2026-07-07.md): - em undo: single-step undo of the last star/flag/unflag/move (1hr window, no stack), built on lib/em-cache.zsh's existing get/set API - em move --recent <ID>...: quick-pick target folder from last 3 destinations, no fzf required - Centralized ALIASES section in em help (single source of truth instead of scattered per-command mentions) Tests: tests/test-em-undo.zsh (16 new tests), tests/test-em-move-restore.zsh (8/8, newly registered), tests/test-em-help-guards.zsh (undo help coverage). Full suite: 80 passed, 0 failed, 1 skipped. mkdocs build --strict clean. Man-page guard: 12/12. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JnVugoVjTHRK41krTTMi8L
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
While implementing Phase 3 of
docs/specs/SPEC-em-ux-refactor-2026-07-07.md, found thatem move <ID> <folder>is currently broken:_em_moveand its adapter_em_hml_movewereeach defined twice (dispatcher + adapter layer), and zsh's last-definition-wins semantics
meant the two live definitions were incompatible — the live
_em_movecalled_em_hml_movewith the numeric message ID in the source-folder position. Every move wouldtry to move a message from a folder named after its own ID.
Same failure class as the
flag/starcase-duplicate bug fixed in PR #493 — this time aduplicate function definition rather than a duplicate case label. Caught by
tests/test-em-move-restore.zsh, which was never wired intotests/run-all.sh(orphaned,like
test-em-ai-switch.zshwas before) — it now passes 8/8 against the fixed source and isregistered.
Kept the multi-ID interface (
em move <FOLDER> <ID>...) since it's whatem move --help,MASTER-DISPATCHER-GUIDE.md, and the orphaned test already documented/expected. Fixed thesame stale
<ID>-first interface (plus a leftover "em flag= alias forem star" line)across
QUICK-REFERENCE.md,MASTER-DISPATCHER-GUIDE.md,REFCARD-EMAIL-DISPATCHER.md(which had both a stale and a correct
em moverow),EMAIL-DISPATCHER-GUIDE.md,EMAIL-COOKBOOK.md, andman/man1/em.1.Phase 3 (bundled per the spec)
em undo— single-step undo of the last star/flag/unflag/move (1hr window, no stack),built on
lib/em-cache.zsh's existing get/set APIem move --recent <ID>...— quick-pick target folder from last 3 destinations, no fzfALIASESsection inem help(single source of truth)Test plan
tests/test-em-move-restore.zsh: 3/8 → 8/8 against fixed source, now registered inrun-all.shtests/test-em-undo.zsh(new, 16 tests): undo cache round-trip, per-action-type reversal,recent-folders dedup/cap/ordering,
em move --recenttests/test-em-help-guards.zsh: addedundoto--helpcoveragemkdocs build --strict: clean