Skip to content

Expand hover setting to allow for key modifier mode#274001

Merged
benvillalobos merged 18 commits intomicrosoft:mainfrom
benvillalobos:bv/hover-ctrl2
Nov 10, 2025
Merged

Expand hover setting to allow for key modifier mode#274001
benvillalobos merged 18 commits intomicrosoft:mainfrom
benvillalobos:bv/hover-ctrl2

Conversation

@benvillalobos
Copy link
Member

@benvillalobos benvillalobos commented Oct 29, 2025 •
edited
Loading

Fixes #72025

Changes Made

  • Modifies existing hover.enabled setting from a boolean to an enum with values [on, off, onKeyboardModifier]
  • When set to onKeyboardModifier, hover only appears when ctrl, cmd, or alt are held down.
    • The key is determined by which key is currently set at Multi Cursor Modifier. If MCM is set to ctrlCmd, hover triggers on alt and vice versa.
multi-cursor.mp4

To Test

  • Set Hover: Enabled to onKeyboardModifier
  • Observe your current setting for Multi Cursor Modifier
  • Observe that hovers do not show unless the opposite value of Multi Cursor Modifier is held down
    • If set to ctrlCmd, hover should only show when alt is held down.
    • If set to alt, hover should show when either ctrl or cmd (macOS) are held down.
  • Set Multi Cursor Modifier to whatever it is not currently
  • Observe that hovers do not show unless the opposite button is pressed
  • Also worth ensuring Hover: Enabled on and off work as expected

Copilot AI reviewed Oct 29, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR converts the editor.hover.enabled setting from a boolean type to a string enumeration with three possible values: 'on', 'off', and 'onKeyboardModifier'. The new 'onKeyboardModifier' option enables conditional hover behavior based on keyboard modifier keys, allowing hover to be shown when the opposite modifier key from the multi-cursor modifier is pressed.

Key Changes:

  • Changed IEditorHoverOptions.enabled type from boolean to 'on' | 'off' | 'onKeyboardModifier'
  • Added backward compatibility logic to convert boolean values to the new string format
  • Introduced shouldShowHover() utility function to determine hover visibility based on modifier keys
  • Updated all usages throughout the codebase to use the new string values

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/vs/editor/common/config/editorOptions.ts Updated IEditorHoverOptions.enabled type definition, default value, schema configuration with enum descriptions, and added backward compatibility conversion
src/vs/monaco.d.ts Updated public API type definition for IEditorHoverOptions.enabled
src/vs/editor/contrib/hover/browser/hoverUtils.ts Added new shouldShowHover() utility function with documentation to determine hover visibility
src/vs/editor/contrib/hover/browser/contentHoverController.ts Integrated shouldShowHover() logic and updated enabled checks from boolean to string comparison
src/vs/editor/contrib/hover/browser/glyphHoverController.ts Integrated shouldShowHover() logic and updated enabled checks from boolean to string comparison
src/vs/workbench/contrib/debug/browser/debugEditorContribution.ts Updated hover enabled checks to use string values ('off', 'on') instead of boolean
src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts Changed hover enabled initialization from true to 'on'
src/vs/workbench/contrib/replNotebook/browser/replEditor.ts Changed hover enabled initialization from true to 'on'
src/vs/editor/test/browser/config/editorConfiguration.test.ts Updated test to use new string values ('on', 'off') instead of boolean
src/vs/editor/contrib/hover/test/browser/hoverUtils.test.ts Added comprehensive test suite for the new shouldShowHover() function

alexdima
alexdima reviewed Oct 30, 2025
View reviewed changes
alexdima
alexdima reviewed Oct 30, 2025
View reviewed changes
Copy link
Member

@alexdima alexdima left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I left a couple of minor comments about how to better migrate and about the description.

@aiday-mar please also review

benvillalobos
benvillalobos commented Oct 30, 2025
View reviewed changes
@aiday-mar
Copy link
Contributor

aiday-mar commented Nov 3, 2025

This looks good! One thing I noticed though is that when the setting is onModifierKeyPressed and I press option the hover does not appear immediately, it only appears after I move the mouse again so the calculation retriggers. I think if we implement this it would make sense to add code to immediately trigger the hover (after a delay potentially) without having to move the mouse again.

alexdima
alexdima approved these changes Nov 7, 2025
View reviewed changes
Copy link
Member

@alexdima alexdima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@benvillalobos benvillalobos added this to the November 2025 milestone Nov 10, 2025
@benvillalobos benvillalobos mentioned this pull request Nov 10, 2025
Closed
@benvillalobos
Copy link
Member Author

benvillalobos commented Nov 10, 2025 •
edited
Loading

@aiday-mar Merging this as-is so the reviews are scoped. I've filed #276558 and will tackle it this milestone

@benvillalobos benvillalobos merged commit a9a261a into microsoft:main Nov 10, 2025
17 checks passed
Kosta-Github added a commit to Kosta-Github/vscode that referenced this pull request Nov 11, 2025
@Kosta-Github
Merge branch 'main' of https://github.com/microsoft/vscode into termi…
8e08ba4 
…nal-tooltips-markdown

* 'main' of https://github.com/microsoft/vscode: (56 commits)
  edits: show diff for sensitive edit confirmations (microsoft#276620)
  Enable Back button on the Manage Accounts picker (microsoft#276622)
  Ignore obsolete chat content parts when loading persisted session (microsoft#276615)
  settings cleanup (microsoft#276602)
  Remove unused `args: any` parameter
  Terminal suggest - include persistent options in suggestions and improve suggestion grouping (microsoft#276409)
  fix selections not being added (microsoft#276600)
  Embed AI search into the existing search view message (microsoft#276586)
  Cleanup some eslint exemptions (microsoft#276581)
  fix microsoft#276579 (microsoft#276590)
  SCM - cleanup some more eslint rules (microsoft#276571)
  Bump gpu types and skip lib check for gpu typing issue
  Fix in smoke tests
  Remove `forChatSessionTypeAndId`
  Fix one more import
  detect `press any/a key` and ask if user wants to send `a` to terminal (microsoft#276554)
  Filter subagent and todo tools from subagent requests (microsoft#276553)
  Expand hover setting to allow for key modifier mode (microsoft#274001)
  Allow partial monacoEnvironment.getWorker/getWorkerUrl
  Update imports
  ...
This was referenced Dec 2, 2025
Closed
Open
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Dec 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@alexdima alexdima alexdima approved these changes

@aiday-mar aiday-mar Awaiting requested review from aiday-mar

@eleanorjboyd eleanorjboyd Awaiting requested review from eleanorjboyd

Assignees

@benvillalobos benvillalobos

Labels

None yet

Projects

None yet

Milestone

November 2025

Development

Successfully merging this pull request may close these issues.

Disable hover but keep Ctrl+Hover

4 participants

@benvillalobos @aiday-mar @alexdima