Skip to content

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

Open
clrudolphi wants to merge 2 commits into
masterfrom
feat/vscode-rename-disambiguation
Open

VS Code: QuickPick disambiguation for ambiguous F16 rename targets#27
clrudolphi wants to merge 2 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
CI's format:check step failed on this new test file.
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