Cycle only running workspaces with Cmd-` (#330) - #331
Merged
Conversation
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012jEwiawXZxVpaKWKBLyzJU
cycleRequested walked every workspace on disk and handed the next one to switchRequested, whose open launches an instance when none holds the pid file. So the workspace someone had just quit was next in creation order and Cmd-` brought it back. Filter the walk by the workspaces otherOpen reports, the same signal the update-install path trusts, and wrap within that subset. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012jEwiawXZxVpaKWKBLyzJU
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.
Fixes #330.
What was wrong
cycleRequestedinAppFeature+Workspaces.swiftwalkedworkspaces.list(), which is every workspace directory on disk, and sent the next one toswitchRequested. That action'sWorkspaceClient.openraises a live instance if one holds the pid file and launches a new one otherwise. So a workspace someone had just quit was still next in creation order, and ⌘` brought it back.CycleShortcut's premise (one window per instance, so the system's own ⌘` has nothing to cycle) holds. The walk upstream of it was the bug.The fix
The walk is narrowed to the current workspace plus
workspaceClient.otherOpen(), the same callupdateInstallTappedalready trusts before swapping the bundle (it checksWorkspaceLock.holderand that the pid is one of ours). The filter runs overknownso creation order is kept, and the wrap-around uses the running subset's count. With nothing else running, ⌘` does nothing rather than launching the first dead workspace.Tests
Three new cases in
WorkspaceCycleTests, each red againstorigin/main's reducer and green with the fix (-only-testing:graphcodeTests/WorkspaceCycleTests, exit 65 → 0):aWorkspaceWithNoWindowIsSteppedOver— on disk and next in order, but not running: skipped, not launched.nothingElseRunningMeansNowhereToGo— every other workspace quit: no switch in either direction.wrappingCountsTheRunningOnesNotTheOnesOnDisk— four on disk, two running: wraps within the running two, both directions.The five existing cases now state that the others are running, which is what they assumed all along.
Gate (from the worktree, private DerivedData)
xcodebuild test(full suite)** TEST FAILED **, 0Failing tests:xcodebuild testonorigin/mainreducer + new testsswiftlint lintswift format lint --strict🤖 Generated with Claude Code
https://claude.ai/code/session_012jEwiawXZxVpaKWKBLyzJU