Go Back / Go Forward over the Explorer's navigation landings - #538
Open
ericwinger wants to merge 6 commits into
Open
Go Back / Go Forward over the Explorer's navigation landings#538ericwinger wants to merge 6 commits into
ericwinger wants to merge 6 commits into
Conversation
VS Code's Go Back only knows about editor documents, and Jasper opens methods in a single reusable preview tab, so it could never step through them — nor did it see the pane moves that Implementors of, Senders of, GemStone Search, Go to Definition or a hierarchy click make. Back and Forward now walk Jasper's own landings: dictionary, class category, class and method, one press per landing, forward tail dropped on a new landing. A landing is stored as a coordinate (session, dictionary, class, side, selector) and recomputed against the stone on the way back, so a recompiled method still resolves and one that has gone is reported and pruned instead of erroring. A cascade — dictionary to class to method — folds into one entry, so Back never needs two presses to leave a method. Bounded at 50. This replaces the URI-keyed editor history the two commands used to walk: every gemstone:// editor activation already drives syncToEditor, so the landing chain subsumes it, and the old module goes. The Actions & Navigation pane at the top of the Explorer draws the trail newest first, with the landing you are on marked, and carries the button row: Go Back, Go Forward, Recent Locations, Refresh, Commit, Abort, a full/selectors-only list toggle, and Open Workspace. Refresh, Commit and Abort move here off the Dictionaries title bar, where they read as acting on dictionaries. The pane is a webview rather than a tree with title-bar actions because VS Code draws a pane's title actions only while it is expanded AND hovered or focused, never while collapsed, and an extension-contributed pane has a 120px minimum body — so a tree could be small or could show its buttons, never both. The webview keeps every button on screen and spends the unavoidable height on the trail. Its message protocol is whitelisted both ways: a press naming a command outside the row is refused, as is a jump to an index outside the live chain. Working inside one class makes every row read TheClass>>…, which in a sidebar-width pane crowds out the only part that differs, so the list toggle drops the class to the dimmed column beside the selector (gemstone.explorer.navigationSelectorsOnly). Also reachable from gemstone:// editor title bars, the Command Palette, and Ctrl+Alt+- / Ctrl+Shift+- while a GemStone editor or Explorer pane has focus. Closes #498 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolutions: - CHANGELOG: the new Go Back / Go Forward entry supersedes the old editor-tab-history one (that history was deleted by this branch); kept main's Remove Method Category entry alongside it. - gemstoneExplorer.ts: kept the navigation command registrations and took main's rewritten per-pane filter comment (the Methods pane now opens VS Code's own find box). - Moved the two navigation test files into the __tests__/explorer/ directory main reorganized them into, fixing their relative paths. - The Actions & Navigation pane's Open Workspace button pointed at gemstone.sessionOpenWorkspace, which main removed; it now runs gemstone.openWorkspace. Caught by the pane's own manifest test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The chain's rules were unit-tested against hand-built landings, but nothing covered the code that produces those landings or walks back to one. The folding rules are written in terms of pairs a pane cascade is assumed to emit, so the Explorer could have emitted a different pair with every unit test still green while Back took two presses per step. Covers, against the real cascade: a dictionary/class/method drill-down folding to the one place it reached, a second method in the same class pushing its own landing, the two sides of a selector staying separate, a category click that keeps the class not counting as a move, and nothing recording without a session. Covers, for the way back: the dictionary re-resolved by name when the symbol list has shifted, a landing from an unselected session declined rather than reconnected behind the user, a vanished dictionary dropped, a removed method still moving the panes to its class and naming what is gone, and the reveals a restore provokes staying out of the chain. Also covers the Recent Locations picker, and that the three navigation commands have handlers rather than only manifest entries — the failure mode where a palette entry throws on invocation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six changes, from Eric's review of the navigation work and from using it: Work with VS Code's history rather than beside it. Reopening a method's tab with VS Code's own Back, or by clicking the tab, now moves our cursor onto that method instead of appending a duplicate, so the two histories stay on the same step; and a press with no GemStone landing left that way falls through to workbench.action.navigateBack/Forward rather than doing nothing. That last part required dropping `enablement` from the two commands: a disabled command swallows its keystroke instead of letting the default binding through, and these borrow VS Code's own Ctrl+Alt+- keys. One chain per session. Switching the selected session switches the trail with it, a session logging out takes its chain away, and a new Clear Navigation History button empties the shown session's by hand. Make the trail a method trail. Dictionaries, class categories and classes are still recorded — Recent Locations lists everything, each row spelled out in full — but they no longer take a row each; one pinned line above the trail names where you are standing, and consecutive coarse landings collapse, so flipping between two dictionaries leaves one entry rather than two. A place is listed once, at its most recent visit, so returning to a method moves its row instead of repeating it. Draw Open Workspace with the notebook codicon the manifest already declares for the command, so the button matches the Logins pane's; likewise Recent Locations and the real history codicon. Keep the Back/Forward keys alive when the last GemStone tab closes. The binding tested `resourceScheme == gemstone`, which needs an ACTIVE EDITOR, and closing the last one leaves none while focus lands in the empty editor group rather than a view — so both arms missed and the keys went dead until you clicked the pane. A third arm covers that gap, guarded by !editorFocus so editing a .ts file still gets VS Code's own Go Back. Give the Explorer container room to be resized. An expanded extension pane costs a 22px header plus a 120px body floor, and a sixth pane took the container to 732px — past what a sidebar on an ordinary screen can give, at which point VS Code stops drawing sashes and NO pane can be resized. Class Categories now starts collapsed, as Hierarchy already did, bringing it back to 612px, and the navigation pane gets a larger share than the smallest. Also fixes a bug the screenshots exposed: revealClass pins the category pane to a class's own category, so the same method carried a different identity depending on whether you clicked it or walked back to it. A class has exactly one category, so it is no longer part of a method's identity — without this the cursor-follow above compares identities that never match. Every function and method added here was mutation-checked: broken in turn and required to fail a test. That found three real gaps — the trail's selectors-only setting was never turned on by any test, nothing asserted the mode is written to settings rather than held in memory, and the "the reveal never got there" branch of a walk was unreached. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tory # Conflicts: # README.md
Every other contributed view carries a contextualTitle, which is what VS Code shows when a pane is dragged out of its own container: without one, "Actions & Navigation" lands among another extension's tabs saying nothing about whose it is. The pane was added while the rule that asserts this was landing on main, so it is the only view in the manifest that shipped without one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Addresses #498.
This is up for review as an approach, not just an implementation. It deliberately
departs from what the issue proposed, and the departure is the thing worth arguing
about — see "The call I'd like reviewed" below.
What it does
Go Back and Go Forward walk the places the GemStone Explorer has taken you —
dictionary, class category, class, and method — one press per landing, with the forward
trail dropped when you go somewhere new. Browser semantics, which are also VS Code's.
A landing is a coordinate (session, dictionary, class, side, selector), not a handle.
It is recomputed against the stone on the way back, so a method that has since been
recompiled still resolves; the dictionary is re-resolved by name, because a commit
elsewhere can shift every symbol-list index. A landing that no longer resolves is reported
and dropped from the trail rather than erroring, so a second press moves on instead of
sticking. The last 50 are kept.
The trail is listed in a new Actions & Navigation pane at the top of the Explorer,
newest first, with the landing you are on marked; click any row to jump to it. The same
list is available from the keyboard as GemStone: Recent Locations….
The call I'd like reviewed
The issue's preferred approach was to push Jasper's landings onto VS Code's own
navigation history, so the native Go Back/Forward in the title bar would just work, and
to treat a Jasper-private stack as the fallback. This does the opposite: it builds a
private chain.
The reason is that VS Code's history is a history of editor documents. Most Explorer
landings are pane moves that open no document at all — selecting a dictionary, a class
category, or a class — and the ones that do open a document open it in a single reusable
preview tab, which VS Code's history does not track as distinct locations. Registering
landings with the native history would have meant opening a real editor for every pane
move, which is a bigger behavioural change than the feature is worth.
The cost of the private chain is a second set of Back/Forward controls next to the
native ones, which is a genuine wart. If the reviewer prefers the native route, the
recording points are all funnelled through one
recordLandingcall, so the destinationis swappable without touching the call sites.
Two smaller calls, both mine, both worth challenging:
Refresh/Commit/Abort moved onto the same row, since they act on the Explorer and its
session rather than the dictionary list they used to sit above. A pane's title-bar
actions are drawn only while the pane is expanded and hovered or focused, so they
vanish exactly when the pane is small enough to be worth keeping; these do not.
method records a single entry, because the coarser two are the cascade passing through
rather than places the user asked to be. Without this, leaving a method costs three
presses of Back.
What is not recorded yet
The issue lists landings this does not cover, and it should not be read as complete:
Everything that opens a
gemstone://editor is covered, because that activation drivesthe Explorer's own sync — so Implementors of, Senders of, GemStone Search, Find Method in
Class, the method-results picker, plain Go to Definition, and post-refactoring
repositioning all record landings without needing their own call sites. The three above
do not go through that path. Happy to add them here or in a follow-up, whichever the
reviewer prefers.
The issue's "with several System Browsers open, Back should act on the one that produced
the landing" is moot: the System Browser is frozen, so several open browsers is not a case
to design for. One global chain, owned by the Explorer.
Testing
Client-only diff, so the fast gate rather than the version matrix.
lint,format:check,compileclean; client 448 files / 7197 tests, server 322,mcp-server 92 — all passing.
the real pane cascade and asserts the landings it actually produces, plus the way
back (dictionary re-resolution, an unselected session declined rather than reconnected,
a vanished dictionary, a removed method), the Recent Locations picker, and that the
three commands have real handlers rather than only manifest entries.
(removing each folding rule, the restore guard, the stale-entry prune, the dictionary
re-resolution, the session guard, the method-gone report, the trail ordering, and the
command registrations) were each verified to fail the suite.
🤖 Generated with Claude Code