From 2c24fee996dfbd5c7b3207a4be3ebd6b127a73d1 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 28 Aug 2024 17:39:31 +0200 Subject: [PATCH] Polish UI of inline-edits toolbar (#226969) * use lenses for accept/discard commands * use lens actions for headless inline ui (and not right-aligned button bar) * fixes https://github.com/microsoft/vscode/issues/226750 * fixes https://github.com/microsoft/vscode-copilot/issues/7628 * fixes https://github.com/microsoft/vscode-copilot/issues/7707 * fixes (maybe) https://github.com/microsoft/vscode-copilot/issues/7547 * fix https://github.com/microsoft/vscode/issues/226966 * graceful handling of headless run --- .../inlineChat/browser/inlineChatActions.ts | 4 +- .../browser/inlineChatController.ts | 15 ++- .../browser/inlineChatStrategies.ts | 92 +++++++++++++++---- .../contrib/inlineChat/common/inlineChat.ts | 1 + .../browser/view/conflictActions.ts | 4 +- .../browser/view/fixedZoneWidget.ts | 1 + 6 files changed, 96 insertions(+), 21 deletions(-) diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatActions.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatActions.ts index 8119cdd08fa..22d6a83fa5d 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatActions.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatActions.ts @@ -11,7 +11,7 @@ import { EmbeddedDiffEditorWidget } from 'vs/editor/browser/widget/diffEditor/em import { EmbeddedCodeEditorWidget } from 'vs/editor/browser/widget/codeEditor/embeddedCodeEditorWidget'; import { EditorContextKeys } from 'vs/editor/common/editorContextKeys'; import { InlineChatController, InlineChatRunOptions } from 'vs/workbench/contrib/inlineChat/browser/inlineChatController'; -import { ACTION_ACCEPT_CHANGES, CTX_INLINE_CHAT_HAS_AGENT, CTX_INLINE_CHAT_HAS_STASHED_SESSION, CTX_INLINE_CHAT_FOCUSED, CTX_INLINE_CHAT_INNER_CURSOR_FIRST, CTX_INLINE_CHAT_INNER_CURSOR_LAST, CTX_INLINE_CHAT_VISIBLE, CTX_INLINE_CHAT_OUTER_CURSOR_POSITION, CTX_INLINE_CHAT_USER_DID_EDIT, CTX_INLINE_CHAT_DOCUMENT_CHANGED, CTX_INLINE_CHAT_EDIT_MODE, EditMode, MENU_INLINE_CHAT_WIDGET_STATUS, CTX_INLINE_CHAT_REQUEST_IN_PROGRESS, CTX_INLINE_CHAT_RESPONSE_TYPE, InlineChatResponseType, ACTION_REGENERATE_RESPONSE, MENU_INLINE_CHAT_CONTENT_STATUS, ACTION_VIEW_IN_CHAT, ACTION_TOGGLE_DIFF, CTX_INLINE_CHAT_CHANGE_HAS_DIFF, CTX_INLINE_CHAT_CHANGE_SHOWS_DIFF, MENU_INLINE_CHAT_ZONE } from 'vs/workbench/contrib/inlineChat/common/inlineChat'; +import { ACTION_ACCEPT_CHANGES, CTX_INLINE_CHAT_HAS_AGENT, CTX_INLINE_CHAT_HAS_STASHED_SESSION, CTX_INLINE_CHAT_FOCUSED, CTX_INLINE_CHAT_INNER_CURSOR_FIRST, CTX_INLINE_CHAT_INNER_CURSOR_LAST, CTX_INLINE_CHAT_VISIBLE, CTX_INLINE_CHAT_OUTER_CURSOR_POSITION, CTX_INLINE_CHAT_USER_DID_EDIT, CTX_INLINE_CHAT_DOCUMENT_CHANGED, CTX_INLINE_CHAT_EDIT_MODE, EditMode, MENU_INLINE_CHAT_WIDGET_STATUS, CTX_INLINE_CHAT_REQUEST_IN_PROGRESS, CTX_INLINE_CHAT_RESPONSE_TYPE, InlineChatResponseType, ACTION_REGENERATE_RESPONSE, MENU_INLINE_CHAT_CONTENT_STATUS, ACTION_VIEW_IN_CHAT, ACTION_TOGGLE_DIFF, CTX_INLINE_CHAT_CHANGE_HAS_DIFF, CTX_INLINE_CHAT_CHANGE_SHOWS_DIFF, MENU_INLINE_CHAT_ZONE, ACTION_DISCARD_CHANGES } from 'vs/workbench/contrib/inlineChat/common/inlineChat'; import { localize, localize2 } from 'vs/nls'; import { Action2, IAction2Options } from 'vs/platform/actions/common/actions'; import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey'; @@ -287,7 +287,7 @@ export class DiscardHunkAction extends AbstractInlineChatAction { constructor() { super({ - id: 'inlineChat.discardHunkChange', + id: ACTION_DISCARD_CHANGES, title: localize('discard', 'Discard'), icon: Codicon.chromeClose, precondition: CTX_INLINE_CHAT_VISIBLE, diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts index 823ea678aa7..18e4a817bfa 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts @@ -39,7 +39,7 @@ import { ChatAgentLocation } from 'vs/workbench/contrib/chat/common/chatAgents'; import { ChatModel, ChatRequestRemovalReason, IChatRequestModel, IChatTextEditGroup, IChatTextEditGroupState, IResponse } from 'vs/workbench/contrib/chat/common/chatModel'; import { IChatService } from 'vs/workbench/contrib/chat/common/chatService'; import { InlineChatContentWidget } from 'vs/workbench/contrib/inlineChat/browser/inlineChatContentWidget'; -import { HunkInformation, Session, StashedSession } from 'vs/workbench/contrib/inlineChat/browser/inlineChatSession'; +import { HunkInformation, HunkState, Session, StashedSession } from 'vs/workbench/contrib/inlineChat/browser/inlineChatSession'; import { InlineChatError } from 'vs/workbench/contrib/inlineChat/browser/inlineChatSessionServiceImpl'; import { EditModeStrategy, HunkAction, IEditObserver, LiveStrategy, PreviewStrategy, ProgressingEditsOptions } from 'vs/workbench/contrib/inlineChat/browser/inlineChatStrategies'; import { CTX_INLINE_CHAT_EDITING, CTX_INLINE_CHAT_REQUEST_IN_PROGRESS, CTX_INLINE_CHAT_RESPONSE_TYPE, CTX_INLINE_CHAT_USER_DID_EDIT, CTX_INLINE_CHAT_VISIBLE, EditMode, INLINE_CHAT_ID, InlineChatConfigKeys, InlineChatResponseType } from 'vs/workbench/contrib/inlineChat/common/inlineChat'; @@ -989,8 +989,21 @@ export class InlineChatController implements IEditorContribution { // ---- controller API showSaveHint(): void { + if (!this._session) { + return; + } + const status = localize('savehint', "Accept or discard changes to continue saving."); this._ui.value.zone.widget.updateStatus(status, { classes: ['warn'] }); + + if (this._ui.value.zone.position) { + this._editor.revealLineInCenterIfOutsideViewport(this._ui.value.zone.position.lineNumber); + } else { + const hunk = this._session.hunkData.getInfo().find(info => info.getState() === HunkState.Pending); + if (hunk) { + this._editor.revealLineInCenterIfOutsideViewport(hunk.getRangesN()[0].startLineNumber); + } + } } acceptInput() { diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatStrategies.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatStrategies.ts index c44f6e6ee7e..28e5b29c7a9 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatStrategies.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatStrategies.ts @@ -8,7 +8,7 @@ import { coalesceInPlace } from 'vs/base/common/arrays'; import { CancellationToken } from 'vs/base/common/cancellation'; import { Emitter, Event } from 'vs/base/common/event'; import { DisposableStore } from 'vs/base/common/lifecycle'; -import { themeColorFromId } from 'vs/base/common/themables'; +import { themeColorFromId, ThemeIcon } from 'vs/base/common/themables'; import { ICodeEditor, IOverlayWidget, IOverlayWidgetPosition, IViewZone, IViewZoneChangeAccessor } from 'vs/editor/browser/editorBrowser'; import { StableEditorScrollState } from 'vs/editor/browser/stableEditorScroll'; import { LineSource, RenderOptions, renderLines } from 'vs/editor/browser/widget/diffEditor/components/diffEditorViewZones/renderLines'; @@ -27,7 +27,7 @@ import { SaveReason } from 'vs/workbench/common/editor'; import { countWords } from 'vs/workbench/contrib/chat/common/chatWordCounter'; import { HunkInformation, Session, HunkState } from 'vs/workbench/contrib/inlineChat/browser/inlineChatSession'; import { InlineChatZoneWidget } from './inlineChatZoneWidget'; -import { CTX_INLINE_CHAT_CHANGE_HAS_DIFF, CTX_INLINE_CHAT_CHANGE_SHOWS_DIFF, CTX_INLINE_CHAT_DOCUMENT_CHANGED, InlineChatConfigKeys, MENU_INLINE_CHAT_ZONE, minimapInlineChatDiffInserted, overviewRulerInlineChatDiffInserted } from 'vs/workbench/contrib/inlineChat/common/inlineChat'; +import { ACTION_TOGGLE_DIFF, CTX_INLINE_CHAT_CHANGE_HAS_DIFF, CTX_INLINE_CHAT_CHANGE_SHOWS_DIFF, CTX_INLINE_CHAT_DOCUMENT_CHANGED, InlineChatConfigKeys, MENU_INLINE_CHAT_ZONE, minimapInlineChatDiffInserted, overviewRulerInlineChatDiffInserted } from 'vs/workbench/contrib/inlineChat/common/inlineChat'; import { assertType } from 'vs/base/common/types'; import { IModelService } from 'vs/editor/common/services/model'; import { performAsyncTextEdit, asProgressiveEdit } from './utils'; @@ -43,6 +43,9 @@ import { generateUuid } from 'vs/base/common/uuid'; import { MenuWorkbenchButtonBar } from 'vs/platform/actions/browser/buttonbar'; import { EditorOption } from 'vs/editor/common/config/editorOptions'; import { Iterable } from 'vs/base/common/iterator'; +import { ConflictActionsFactory, IContentWidgetAction } from 'vs/workbench/contrib/mergeEditor/browser/view/conflictActions'; +import { observableValue } from 'vs/base/common/observable'; +import { IMenuService, MenuItemAction } from 'vs/platform/actions/common/actions'; export interface IEditObserver { start(): void; @@ -216,8 +219,10 @@ type HunkDisplayData = { decorationIds: string[]; - viewZoneId: string | undefined; - viewZone: IViewZone; + diffViewZoneId: string | undefined; + diffViewZone: IViewZone; + + lensActionsViewZoneIds?: string[]; distance: number; position: Position; @@ -257,6 +262,7 @@ export class LiveStrategy extends EditModeStrategy { private readonly _ctxCurrentChangeShowsDiff: IContextKey; private readonly _progressiveEditingDecorations: IEditorDecorationsCollection; + private readonly _lensActionsFactory: ConflictActionsFactory; private _editCount: number = 0; constructor( @@ -268,6 +274,8 @@ export class LiveStrategy extends EditModeStrategy { @IEditorWorkerService protected readonly _editorWorkerService: IEditorWorkerService, @IAccessibilityService private readonly _accessibilityService: IAccessibilityService, @IConfigurationService private readonly _configService: IConfigurationService, + @IMenuService private readonly _menuService: IMenuService, + @IContextKeyService private readonly _contextService: IContextKeyService, @ITextFileService textFileService: ITextFileService, @IInstantiationService instaService: IInstantiationService ) { @@ -276,6 +284,7 @@ export class LiveStrategy extends EditModeStrategy { this._ctxCurrentChangeShowsDiff = CTX_INLINE_CHAT_CHANGE_SHOWS_DIFF.bindTo(contextKeyService); this._progressiveEditingDecorations = this._editor.createDecorationsCollection(); + this._lensActionsFactory = this._store.add(new ConflictActionsFactory(this._editor)); } @@ -487,43 +496,93 @@ export class LiveStrategy extends EditModeStrategy { afterLineNumber: -1, heightInLines: result.heightInLines, domNode, - ordinal: 50000 + 1 // more than https://github.com/microsoft/vscode/blob/bf52a5cfb2c75a7327c9adeaefbddc06d529dcad/src/vs/workbench/contrib/inlineChat/browser/inlineChatZoneWidget.ts#L42 + ordinal: 50000 + 2 // more than https://github.com/microsoft/vscode/blob/bf52a5cfb2c75a7327c9adeaefbddc06d529dcad/src/vs/workbench/contrib/inlineChat/browser/inlineChatZoneWidget.ts#L42 }; const toggleDiff = () => { const scrollState = StableEditorScrollState.capture(this._editor); changeDecorationsAndViewZones(this._editor, (_decorationsAccessor, viewZoneAccessor) => { assertType(data); - if (!data.viewZoneId) { + if (!data.diffViewZoneId) { const [hunkRange] = hunkData.getRangesN(); viewZoneData.afterLineNumber = hunkRange.startLineNumber - 1; - data.viewZoneId = viewZoneAccessor.addZone(viewZoneData); + data.diffViewZoneId = viewZoneAccessor.addZone(viewZoneData); overlay?.updateExtraTop(result.heightInLines); } else { - viewZoneAccessor.removeZone(data.viewZoneId!); + viewZoneAccessor.removeZone(data.diffViewZoneId!); overlay?.updateExtraTop(0); - data.viewZoneId = undefined; + data.diffViewZoneId = undefined; } }); - this._ctxCurrentChangeShowsDiff.set(typeof data?.viewZoneId === 'string'); + this._ctxCurrentChangeShowsDiff.set(typeof data?.diffViewZoneId === 'string'); scrollState.restore(this._editor); }; - const overlay = this._showOverlayToolbar + const overlay = this._showOverlayToolbar && false ? this._instaService.createInstance(InlineChangeOverlay, this._editor, hunkData) : undefined; + + let lensActions: DisposableStore | undefined; + const lensActionsViewZoneIds: string[] = []; + + if (this._showOverlayToolbar && hunkData.getState() === HunkState.Pending) { + + lensActions = new DisposableStore(); + + const menu = this._menuService.createMenu(MENU_INLINE_CHAT_ZONE, this._contextService); + const makeActions = () => { + const actions: IContentWidgetAction[] = []; + const tuples = menu.getActions(); + for (const [, group] of tuples) { + for (const item of group) { + if (item instanceof MenuItemAction) { + + let text = item.label; + + if (item.id === ACTION_TOGGLE_DIFF) { + text = item.checked ? 'Hide Changes' : 'Show Changes'; + } else if (ThemeIcon.isThemeIcon(item.item.icon)) { + text = `$(${item.item.icon.id}) ${text}`; + } + + actions.push({ + text, + tooltip: item.tooltip, + action: async () => item.run(), + }); + } + } + } + return actions; + }; + + const obs = observableValue(this, makeActions()); + lensActions.add(menu.onDidChange(() => obs.set(makeActions(), undefined))); + lensActions.add(menu); + + lensActions.add(this._lensActionsFactory.createWidget(viewZoneAccessor, + hunkRanges[0].startLineNumber - 1, + obs, + lensActionsViewZoneIds + )); + } + const remove = () => { changeDecorationsAndViewZones(this._editor, (decorationsAccessor, viewZoneAccessor) => { assertType(data); for (const decorationId of data.decorationIds) { decorationsAccessor.removeDecoration(decorationId); } - if (data.viewZoneId) { - viewZoneAccessor.removeZone(data.viewZoneId); + if (data.diffViewZoneId) { + viewZoneAccessor.removeZone(data.diffViewZoneId); } data.decorationIds = []; - data.viewZoneId = undefined; + data.diffViewZoneId = undefined; + + data.lensActionsViewZoneIds?.forEach(viewZoneAccessor.removeZone); + data.lensActionsViewZoneIds = undefined; + lensActions?.dispose(); }); overlay?.dispose(); @@ -548,8 +607,9 @@ export class LiveStrategy extends EditModeStrategy { data = { hunk: hunkData, decorationIds, - viewZoneId: '', - viewZone: viewZoneData, + diffViewZoneId: '', + diffViewZone: viewZoneData, + lensActionsViewZoneIds, distance: myDistance, position: hunkRanges[0].getStartPosition().delta(-1), acceptHunk, diff --git a/src/vs/workbench/contrib/inlineChat/common/inlineChat.ts b/src/vs/workbench/contrib/inlineChat/common/inlineChat.ts index d656e6c3f25..f4c87af0d22 100644 --- a/src/vs/workbench/contrib/inlineChat/common/inlineChat.ts +++ b/src/vs/workbench/contrib/inlineChat/common/inlineChat.ts @@ -116,6 +116,7 @@ export const CTX_INLINE_CHAT_RESPONSE_TYPE = new RawContextKey { this.widgetDomNode.style.height = `${height}px`; },