editor hover: add MenuId for marker hover status bar actions#303454
editor hover: add MenuId for marker hover status bar actions#303454
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new menu extension point for the editor marker hover status bar, enabling workbench-contributed actions (e.g., inline chat “Fix”) to appear alongside quick fixes in the problems hover.
Changes:
- Introduces
MenuId.MarkerHoverStatusBaras a new contribution point. - Updates
MarkerHoverParticipantto query/render enabled menu actions in the hover status bar and suppresses “No quick fixes available” when actions exist. - Registers
FixDiagnosticsActioninto the new menu and adjusts hover status bar icon sizing via CSS.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/vs/workbench/contrib/inlineChat/browser/inlineChatActions.ts | Contributes the inline chat diagnostics fix action to the new marker-hover status bar menu. |
| src/vs/platform/actions/common/actions.ts | Adds MenuId.MarkerHoverStatusBar to the shared menu ID registry. |
| src/vs/editor/contrib/hover/browser/markerHoverParticipant.ts | Renders menu-contributed actions in the marker hover status bar and adjusts placeholder behavior. |
| src/vs/base/browser/ui/hover/hoverWidget.css | Ensures status bar action icons inherit font size for consistent alignment. |
Comments suppressed due to low confidence (2)
src/vs/editor/contrib/hover/browser/markerHoverParticipant.ts:320
- If getCodeActions() rejects (e.g., due to provider registry changes causing cancellation), the current rejection handler only calls onUnexpectedError, which means menu-contributed actions never get rendered and the placeholder may stay stuck. Consider using a custom rejection handler that still renders menu actions (and updates/hides the placeholder as needed), while only logging truly unexpected errors.
const codeActionsPromise = this.getCodeActions(markerHover.marker);
disposables.add(toDisposable(() => codeActionsPromise.cancel()));
codeActionsPromise.then(actions => {
updatePlaceholderDisposable.dispose();
this.recentMarkerCodeActionsInfo = { marker: markerHover.marker, hasCodeActions: actions.validActions.length > 0 };
if (!this.recentMarkerCodeActionsInfo.hasCodeActions) {
actions.dispose();
if (menuActions.length === 0) {
quickfixPlaceholderElement.textContent = nls.localize('noQuickFixes', "No quick fixes available");
} else {
quickfixPlaceholderElement.style.display = 'none';
}
renderMenuActions();
return;
}
quickfixPlaceholderElement.style.display = 'none';
let showing = false;
disposables.add(toDisposable(() => {
if (!showing) {
actions.dispose();
}
}));
context.statusBar.addAction({
label: nls.localize('quick fixes', "Quick Fix..."),
commandId: quickFixCommandId,
run: (target) => {
showing = true;
const controller = CodeActionController.get(this._editor);
const elementPosition = dom.getDomNodePagePosition(target);
// Hide the hover pre-emptively, otherwise the editor can close the code actions
// context menu as well when using keyboard navigation
context.hide();
controller?.showCodeActions(markerCodeActionTrigger, actions, {
x: elementPosition.left,
y: elementPosition.top,
width: elementPosition.width,
height: elementPosition.height
});
}
});
const aiCodeAction = actions.validActions.find(action => action.action.isAI);
if (aiCodeAction) {
context.statusBar.addAction({
label: aiCodeAction.action.title,
commandId: aiCodeAction.action.command?.id ?? '',
iconClass: ThemeIcon.asClassName(Codicon.sparkle),
run: () => {
const controller = CodeActionController.get(this._editor);
controller?.applyCodeAction(aiCodeAction, false, false, ApplyCodeActionReason.FromProblemsHover);
}
});
}
renderMenuActions();
// Notify that the contents have changed given we added
// actions to the hover
// https://github.com/microsoft/vscode/issues/250424
context.onContentsChanged();
}, onUnexpectedError);
} else {
src/vs/editor/contrib/hover/browser/markerHoverParticipant.ts:232
- MenuItemAction.run() returns a Promise, but this handler neither focuses the hovered editor (so EditorAction2 commands may execute against a different focused/active editor) nor handles rejections. Consider focusing the editor before setting the selection, and ensure any rejection from action.run() is handled (e.g., forward to onUnexpectedError) to avoid unhandled promise rejections.
run: () => {
context.hide();
this._editor.setSelection(Range.lift(markerHover.range));
action.run();
}
| } else { | ||
| quickfixPlaceholderElement.style.display = 'none'; | ||
| } | ||
| renderMenuActions(); |
There was a problem hiding this comment.
When there are no quick fixes, menu actions are added asynchronously but the hover is not notified that contents changed. Since the code calls context.onContentsChanged() in the success path after adding actions, the no-code-actions early-return should also notify (after renderMenuActions) so the hover can relayout and ensure the newly added actions are visible.
This issue also appears in the following locations of the same file:
- line 255
- line 228
| renderMenuActions(); | |
| renderMenuActions(); | |
| context.onContentsChanged(); |
| // Menu-contributed actions (e.g. fix with inline chat) | ||
| const menuActions: MenuItemAction[] = []; | ||
| for (const [, actions] of this._menuService.getMenuActions(MenuId.MarkerHoverStatusBar, this._contextKeyService)) { | ||
| for (const action of actions) { | ||
| if (action instanceof MenuItemAction && action.enabled) { | ||
| menuActions.push(action); | ||
| } | ||
| } | ||
| } | ||
| const renderMenuActions = () => { | ||
| for (const action of menuActions) { | ||
| context.statusBar.addAction({ | ||
| label: action.label, | ||
| commandId: action.id, | ||
| iconClass: action.class, | ||
| run: () => { | ||
| context.hide(); | ||
| this._editor.setSelection(Range.lift(markerHover.range)); | ||
| action.run(); | ||
| } | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
This adds a new menu-based extension point to the marker hover UI, but there doesn’t appear to be test coverage validating that menu actions render (including the interaction with the "No quick fixes available" placeholder). Consider adding a unit test under src/vs/editor/contrib/hover/test/browser that stubs IMenuService.getMenuActions() and asserts the status bar renders the contributed action(s) and suppresses the placeholder when appropriate.
This PR introduces a
MenuId.MarkerHoverStatusBarso that workbench contributions (like inline chat's Fix action) can add actions to the marker hover status bar via the menu system.Changes
MenuId.MarkerHoverStatusBaradded toactions.ts- a new extension point for the marker hover status barMarkerHoverParticipantqueries this menu viaIMenuService.getMenuActions()and renders enabled actions in the hover status bar, after Quick Fix actionsFixDiagnosticsActionregisters itself in the new menu withprecondition: null(the hover already guarantees a diagnostic is present) and appropriatewhenguards (CTX_FIX_DIAGNOSTICS_ENABLED,CTX_INLINE_CHAT_FILE_BELONGS_TO_CHAT.negate(), plusCTX_INLINE_CHAT_V2_ENABLEDauto-added byAbstractInlineChatAction)attachDiagnosticsfinds the correct diagnosticfont-size: inheritfix for hover status bar action iconsFixes #300857