fix: use setupDelayedHover for settings indicator hovers to support Ctrl+K I#304990
Merged
rzhao271 merged 3 commits intomicrosoft:mainfrom Apr 24, 2026
Merged
Conversation
When ensureConfigurationForDocument is called and no visible text editor is found for the document, getFormattingOptions returns undefined and the method returns early without sending any configuration including user preferences like preferTypeOnlyAutoImports to the TS server. This causes source.addMissingImports to ignore the user's preferTypeOnlyAutoImports setting. Fix by falling back to undefined formatting options when no visible editor is found, ensuring user preferences are always sent. Closes microsoft#272479
…trl+K I Switch all indicator hovers from showInstantHover+addHoverDisposables to setupDelayedHover with setupKeyboardEvents. This registers hovers in _delayedHovers, allowing _showAndFocusHoverForActiveElement to find and trigger the correct hover when Ctrl+K I is pressed while a setting indicator (Modified Elsewhere, Workspace Trust, etc.) is focused. Previously, Ctrl+K I would walk up the DOM tree and find a parent element hover (showing the setting ID) instead of the indicator hover. Fixes microsoft#257903
Contributor
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @rzhao271Matched files:
|
rzhao271
approved these changes
Apr 17, 2026
Collaborator
rzhao271
left a comment
rzhao271
left a comment
There was a problem hiding this comment.
LGTM as a refactor. Ctrl+K I is still not supported, but I can close the original issue as not planned.
roblourens
approved these changes
Apr 17, 2026
rzhao271
approved these changes
Apr 24, 2026
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
3 participants
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.
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
When a settings editor indicator (e.g. "Modified Elsewhere", "Workspace Trust", "Not Synced", "Default value changed") is focused via Tab, pressing Ctrl+K I does not show that indicator's hover. Instead, the setting ID hover from a parent element is shown.
This happens because the indicators use
showInstantHoverwith manual event handlers (addHoverDisposables), which does not register the hover in the hover service's_delayedHoversmap. When_showAndFocusHoverForActiveElementis invoked by Ctrl+K I, it walks up the DOM tree and finds a parent element's hover instead.Closes #257903
What is the new behavior?
All setting indicator hovers now use
setupDelayedHoverwithsetupKeyboardEvents: true, which:_delayedHoversso_showAndFocusHoverForActiveElementfinds the correct hover for Ctrl+K IsetupKeyboardEventsThe now-unused
addHoverDisposablesmethod and its related imports (RunOnceScheduler,IHoverWidget) are removed.Additional context
This affects all setting indicators: Modified Elsewhere, Workspace Trust, Sync Ignored, Preview, Experimental, Advanced, Default Override, Policy, and Application Setting indicators.