Rewrite tab selection tracking for sheets#1
Merged
Conversation
Sheet tracking is required for several reasons: 1. There are non-text sheets (HTML and images) that do not have views and thus do not show up in various view-specific APIs. 2. You can select multiple sheets, which the default behavior supports, and we do not want to regress here, so we have to track all selected sheets at a time. This requires much more complex tracking code and persistence because we cannot simply attach a timestamp to a view's settings object anymore. As such, we now store and synchronize our recorded selection history inside the window's settings object, keyed by the group id (which is stable across restarts) and by computing a stable sheet id, and need to implement various cleanup logic, including code that de-duplicates sheets from a previous multi-selection when it has later been selected alone, and much more. Another annoying part is that EventListener hooks do not fire for sheets, which is why we implement a polling mechanism to synchronize selections also when those sheets are selected.
Unfortunately, contexts just do not work for non-view sheets, so I have to resort to disabling the safeguard context that checks if the necessary system libraries can be opened. Instead, we defer to the default commands dynamically during command execution so that tab switching still works in some capacity.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR rewrites TabStack’s “most recently used” tab switching to track sheet selections (including non-view sheets like HTML/image sheets and multi-selections) by persisting per-group selection history in window settings and restoring prior selections on close/abort.
Changes:
- Introduces a window-settings-backed selection history keyed by group, plus pruning/deduplication and polling-based synchronization.
- Replaces view-based MRU tracking with sheet-identity-based entry collection and selection application.
- Updates quick panel session handling, key bindings, and docs to reflect the new sheet-based behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates user-facing behavior description (multi-selection + restore previous selection). |
| plugin/state.py | Refactors per-window session state to track sheet selections and new pollers. |
| plugin/sheets.py | Adds stable-ish sheet identity + selection (re)application helpers. |
| plugin/session.py | New quick panel session workflow for preview/commit/cancel using selections. |
| plugin/mru.py | Builds quick panel entries from selection history + live sheets. |
| plugin/listener.py | Switches activation/close handling to selection history and polling sync. |
| plugin/history.py | Adds persisted selection history model, sync, pruning, and poller. |
| plugin/commands.py | Updates show_tab_stack command to use selection history and new session module. |
| plugin/captions.py | Updates caption generation to support multi-sheet selections. |
| plugin/init.py | Ensures new pollers are stopped on plugin unload. |
| Default.sublime-keymap | Adjusts bindings/args for forward switching and quick-panel navigation contexts. |
| AGENTS.md | Updates repository notes and verification guidance for the new architecture. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
FichteFoll
force-pushed
the
feature/sheet-selection
branch
from
076f75d to
16d1d83
Compare
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
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.
Sheet tracking is required for several reasons:
and thus do not show up in various view-specific APIs.
and we do not want to regress here, so we have to track all selected
sheets at a time.
This requires much more complex tracking code and persistence because we
cannot simply attach a timestamp to a view's settings object anymore.
As such, we now store and synchronize our recorded selection history
inside the window's settings object, keyed by the group id (which is
stable across restarts) and by computing a stable sheet id, and need to
implement various cleanup logic, including code that de-duplicates
sheets from a previous multi-selection when it has later been selected
alone, and much more.
Another annoying part is that EventListener hooks do not fire for
sheets, which is why we implement a polling mechanism to synchronize
selections also when those sheets are selected.