Fix window move: insert at boundaries instead of swap #3
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "1-fix-window-move-insert"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
<and>with external scripts using absolute indicesChanges
.tmux.conf: Replace inlineswap-windowoverrides withrun-shellcalls to external scriptsscripts/jump-to-front.sh: New —>at last position bubbles window to position 0scripts/jump-to-back.sh: New —<at position 0 bubbles window to last positionTest Plan
prefix >= one step rightprefix <= one step leftprefix >= jump to front, everything shifts rightprefix <= jump to back, everything shifts leftReview Checklist
Related
-dflag, broke repeat behavior)PR #3 Review
DOMAIN REVIEW
Tech stack: POSIX shell scripts + tmux configuration (dotfiles repo).
Shell correctness: Both scripts are clean POSIX
/bin/sh. Variables are properly quoted, arithmetic uses$((...)), no word-splitting or globbing risks. No shellcheck violations detected.Tmux integration: The approach of using
run-shellto call external scripts is sound. The swap-then-select pattern correctly maintains focus on the moved window.Non-contiguous window index assumption: Both scripts assume window indices are contiguous (0, 1, 2, ..., N). The normal-case swap uses arithmetic (
current - 1/current + 1), and the boundary-case loop increments/decrements by 1. If windows have gaps in numbering (e.g., 0, 1, 5 after closing window 2), swapping with a non-existent index will fail silently in tmux (swap-window to a non-existent target creates the target, effectively moving the window). This is actually safe in practice -- tmux'sswap-windowto a non-existent target index just moves the window there. However, the bubble loop would stop working correctly because it would leave gaps. Worth understanding but likely acceptable for a personal dotfiles repo whererenumber-windowsis typically on.Repeat mode removed: The old bindings had
-r(repeat prefix). The newrun-shellbindings do not have-r, so each move requires pressing prefix again. This is a deliberate tradeoff noted in the PR body (PR #2's-dremoval broke repeat). Acceptable -- the boundary wrap behavior is the primary goal.BLOCKERS
None.
This is a dotfiles/tooling repo. The BLOCKER criteria (test coverage for new functionality, input validation, secrets, DRY in auth paths) do not apply in the traditional sense:
NITS
DRY between scripts:
jump-to-back.shandjump-to-front.share structural mirrors. Could be a singlemove-window.shaccepting a direction argument. For two 18-line scripts in a dotfiles repo, this is purely cosmetic.Non-contiguous index resilience: If you ever turn off
renumber-windows, the boundary bubble loop could behave unexpectedly. A more robust approach would iterate over actual window indices fromtmux list-windows -F '#{window_index}'rather than arithmetic increment. Low priority for dotfiles.No shebang path portability note:
#!/bin/shis fine for Linux. If this dotfiles repo is ever used on macOS where/bin/shis a thin zsh wrapper, behavior is identical for this code, but worth noting.SOP COMPLIANCE
1-fix-window-move-insertreferences issue #1)PROCESS OBSERVATIONS
-dflag, broke repeat). Good iteration discipline -- superseded the broken PR rather than force-pushing over it.VERDICT: APPROVED