Refuse x and c unless the session list has focus - #11
Merged
Merged
Conversation
sessions.go held creating, selecting and closing. The next commit adds a resolver beside the other two and would take the file past the 200-line rule, so the split comes first and on its own — a refactor is unreviewable once a behaviour change is hiding in it. closing.go is named for the category rather than its occupant. Deleting a worktree, when it exists, belongs here too. The doc comment travels with the function and no copy stays behind; sessions.go's header now says where closing went rather than continuing to claim it.
dashboardKey routed both keys whatever column had focus, while dashboardSession resolves through listIx. Pressing x on the projects list therefore closed that project's newest session — a row the keyboard was not driving. The row was never invisible: cursorMarker keeps a dimmed cursor on the unfocused column deliberately, and there is a test named for that. What was missing was the focus. sectionLeft already scopes the arrow keys by it and states the reason — the focused column is drawn with an accent border, and a key that reaches across makes that border a lie. The footer said the same by omitting both keys there, so x was destructive and undocumented in the same place. Closing is where it costs most, because the arrows are reversible and closing is not. focusedSession is where the rule lives, and the refusal says why: jumpToSession settled that a key which silently does nothing only invites a second press. Closing the last session is the same invariant from the other side, and it goes here rather than in its own commit because the refusal above depends on it. focusContent refuses to focus an empty list, but closing reached that state anyway — the cursor sat in a column with nothing in it, and the next x refused in silence because there was no session for the notice to name. Focus now returns to the projects column when a project runs out of sessions. The connect key resolves into a variable rather than inline, because focusedSession writes its notice into m and Go does not order a method's receiver against a call in its own argument list. The close test now asserts the notice names the worktree directory rather than merely being non-empty. Keeping the worktree is only useful if you are told where it went, and that promise had no test at all.
Closing keeps the worktree, which is the right default and currently the only one. Nothing in Deck removes what it keeps, so a machine accumulates a tree and a session/* branch per closed isolated session, and git by hand is the only way out. The entry settles the shape: x opens a modal with both outcomes rather than growing a second key, Close stays the default, and the Delete row carries the gitx.Diff summary so the confirmation states a fact rather than a warning. Four decisions go with it, and the one most likely to be softened later is that nothing is forced — git worktree remove refuses a dirty tree and git branch -d refuses unmerged commits, and both refusals are the answer. Verified rather than assumed: in a throwaway repository, removing a dirty worktree gives "contains modified or untracked files, use --force", while a clean worktree whose branch holds unmerged commits removes fine and then branch -d refuses. That is exactly the gate-and-best-effort split the entry describes. No code changes.
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.
Three commits, each green on its own.
closing.go, a pure move, so the refactor is not hiding under the behaviour change that follows.xandcunless the session list has focus —dashboardKeyrouted both keys whatever column had focus, whiledashboardSessionresolves throughlistIx, soxon the projects list closed that project's newest session. The row was never invisible —cursorMarkerkeeps a dimmed cursor deliberately — but the keyboard was not driving it, the footer never offeredxthere, andsectionLeftalready scopes the arrow keys by focus for the same stated reason.The guard commit also releases focus when a project's last session closes.
focusContentrefuses to focus an empty list, and closing reached that state from the other side, leavingfocusedSessionwith nothing to name and its refusal silent. The two are one invariant, so they land together.The close test now asserts the notice names the worktree directory rather than merely being non-empty. Keeping the worktree is only useful if you are told where it went, and that promise had no test.