Skip to content

VS Code: QuickPick disambiguation for ambiguous F16 rename targets - #27

Merged
clrudolphi merged 6 commits into
masterfrom
feat/vscode-rename-disambiguation
Jul 5, 2026
Merged

VS Code: QuickPick disambiguation for ambiguous F16 rename targets#27
clrudolphi merged 6 commits into
masterfrom
feat/vscode-rename-disambiguation

Conversation

@clrudolphi

Copy link
Copy Markdown
Collaborator

Summary

  • VS Code's prepareRename previously returned null whenever the cursor's step text matched more than one step-binding attribute, silently suppressing rename with no explanation to the user.
  • The Visual Studio client already handles this case via a custom disambiguation dialog, backed by the server's existing reqnroll/renameTargets (enumerate candidates) and reqnroll/selectRenameTarget (record the user's pick for the next textDocument/rename) custom requests — this plumbing is generic, not VS-specific.
  • This PR adds a VS Code-idiomatic equivalent entirely on the client: a RenameMiddleware.prepareRename override (src/VSCode/src/renameDisambiguation.ts) that queries reqnroll/renameTargets first. When 0 or 1 candidates are returned, it delegates straight through to VS Code's native prepareRename (no behavior change). When 2+ candidates are returned, it shows a vscode.window.showQuickPick listing the ambiguous binding attributes, sends reqnroll/selectRenameTarget with the chosen index (using version: 0, matching how the Visual Studio client and the server's RenameSessionManager key sessions), and then proceeds with the normal rename flow so VS Code's native rename input box still opens.
  • No server-side changes were required — the existing custom requests already covered everything needed to expose the ambiguity.

Test plan

  • npm run compile passes cleanly
  • npm run lint passes cleanly
  • npm test (real Extension Development Host run via @vscode/test-electron) — 23 passing, 1 pending (documents that driving the interactive QuickPick itself isn't automatable headlessly; the dismiss-picker and pass-through branches are covered)
  • No server-side (Reqnroll.IdeSupport.LSP.Server) code touched, so no LSP spec regression risk
  • Manual VS Code Extension Host validation recommended before merge: open a .cs file with two [Given]/[When]/[Then] attributes on the same method (or two bindings resolving to the same feature step), invoke rename (F2 or the "Rename Step" command), confirm the QuickPick lists both attributes, and confirm choosing one renames only that binding while dismissing the picker cancels rename cleanly.

Closes #10

VS Code's prepareRename previously returned null for a step whose text
matches multiple binding attributes, silently suppressing rename with no
explanation -- unlike the Visual Studio client, which already shows a
disambiguation dialog for the same case via the reqnroll/renameTargets and
reqnroll/selectRenameTarget custom requests.

That server-side plumbing is already generic (not VS-specific), so this adds
a VS Code-idiomatic equivalent purely on the client: a RenameMiddleware
override intercepts textDocument/prepareRename, queries
reqnroll/renameTargets, and when more than one binding matches, shows a
QuickPick and sends reqnroll/selectRenameTarget with the user's choice before
delegating to the standard rename flow. Zero/one-target cases pass straight
through to VS Code's native prepareRename, so normal rename UX is unchanged.

Closes #10
clrudolphi and others added 5 commits July 3, 2026 04:49
CI's format:check step failed on this new test file.
Reqnroll.IdeSupport.LSP.Server's BuildConnector target already builds
Connector.csproj transitively (with its own global properties) before
the server compiles. Building it again as an independent solution node
races that same build's CustomAfterBuild target — both instances
RemoveDir/Copy into the identical Connectors\ output folder under VS's
parallel project builds, intermittently failing with "directory not
empty". Exclude the standalone node from Debug builds; Release keeps
it for VSIX/CI packaging.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
FindBindingsAtFeatureStep gated candidates on MatchResult.HasDefined,
but an ambiguous step's items are typed Ambiguous, not Defined —
excluding exactly the steps this picker exists to disambiguate. Both
reqnroll/renameTargets (empty picker) and prepareRename (null, which
the server's null->throw wiring surfaces as a visible client error
popup) were affected. Now also accept HasAmbiguous items.

Also distinguish otherwise-identical picker entries by appending the
implementing method (shortened to Class.Method, dropping the shared
namespace prefix that was pushing the only distinguishing part past
the picker's visible width).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
VS Code's Uri.toString() percent-encodes the Windows drive-letter
colon (file:///c%3A/...), which reqnroll/selectRenameTarget carried
verbatim as a raw string. HandleRenameAsync's session lookup instead
keyed off DocumentUri.ToString(), which round-trips unescaped
(file:///c:/...) — the two never matched, so TryConsume always missed
and HandleRenameAsync silently fell back to the first ambiguous
candidate regardless of what the user selected in the picker.

Type SelectRenameTargetParams.Uri as DocumentUri (matching every other
incoming-URI field in the codebase) so both sides of the session key
go through the same normalizing round-trip, closing the gap at its
source. RenameSessionManager.NormalizeUri also now unescapes
percent-encoding as defense-in-depth.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VS Code: F16 (Rename Step) has no multi-attribute disambiguation UI

1 participant