diff --git a/src/vs/editor/browser/widget/diffEditor/diffEditorWidget.ts b/src/vs/editor/browser/widget/diffEditor/diffEditorWidget.ts index f252b68138cc..854d67106d83 100644 --- a/src/vs/editor/browser/widget/diffEditor/diffEditorWidget.ts +++ b/src/vs/editor/browser/widget/diffEditor/diffEditorWidget.ts @@ -633,6 +633,10 @@ export class DiffEditorWidget extends DelegatingEditor implements IDiffEditor { get renderSideBySide(): boolean { return this._options.renderSideBySide.get(); } + get renderSideBySideInAutomaticMode(): IObservable { return this._options.renderSideBySideInAutomaticMode; } + + get temporaryInlineMode(): IObservable { return this._options.temporaryInlineMode; } + resetWidthBasedLayout(): void { this._options.resetWidthBasedLayout(); } diff --git a/src/vs/editor/browser/widget/multiDiffEditor/multiDiffEditorWidget.ts b/src/vs/editor/browser/widget/multiDiffEditor/multiDiffEditorWidget.ts index 24f11fe4ccd1..525c02bbc058 100644 --- a/src/vs/editor/browser/widget/multiDiffEditor/multiDiffEditorWidget.ts +++ b/src/vs/editor/browser/widget/multiDiffEditor/multiDiffEditorWidget.ts @@ -12,7 +12,7 @@ import { URI } from '../../../../base/common/uri.js'; import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js'; import { IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; import { Range } from '../../../common/core/range.js'; -import { IDiffEditorOptions } from '../../../common/config/editorOptions.js'; +import { DiffEditorViewMode, IDiffEditorOptions } from '../../../common/config/editorOptions.js'; import { IDiffEditor } from '../../../common/editorCommon.js'; import { IMultiDiffResourceId } from '../../../common/multiDiffEditor.js'; import { ICodeEditor } from '../../editorBrowser.js'; @@ -115,6 +115,15 @@ export class MultiDiffEditorWidget extends Disposable { }, undefined); } + public setViewMode(mode: DiffEditorViewMode): void { + const currentOptions = this._diffLayoutOptions.get(); + const wasAutomatic = currentOptions?.renderSideBySide === true && currentOptions.useInlineViewWhenSpaceIsLimited === true; + this.setRenderSideBySide(mode !== 'inline', { useInlineViewWhenSpaceIsLimited: mode === 'automatic' }); + if (mode === 'automatic' && !wasAutomatic) { + this.resetWidthBasedLayout(); + } + } + public toggleRenderSideBySide(): void { this.setRenderSideBySide(!(this._diffLayoutOptions.get()?.renderSideBySide ?? true)); } @@ -164,6 +173,10 @@ export class MultiDiffEditorWidget extends Disposable { return this._widgetImpl.get().getScopedInstantiationService(); } + public resetWidthBasedLayout(): void { + this._widgetImpl.get().resetWidthBasedLayout(); + } + public findDocumentDiffItem(resource: URI): IDocumentDiffItem | undefined { return this._widgetImpl.get().findDocumentDiffItem(resource); } diff --git a/src/vs/editor/browser/widget/multiDiffEditor/multiDiffEditorWidgetImpl.ts b/src/vs/editor/browser/widget/multiDiffEditor/multiDiffEditorWidgetImpl.ts index 707977dd82b4..bf914a6462a1 100644 --- a/src/vs/editor/browser/widget/multiDiffEditor/multiDiffEditorWidgetImpl.ts +++ b/src/vs/editor/browser/widget/multiDiffEditor/multiDiffEditorWidgetImpl.ts @@ -13,6 +13,7 @@ import { ContextKeyValue, IContextKeyService } from '../../../../platform/contex import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js'; import { ServiceCollection } from '../../../../platform/instantiation/common/serviceCollection.js'; import { ILogService } from '../../../../platform/log/common/log.js'; +import { bindContextKey } from '../../../../platform/observable/common/platformObservableUtils.js'; import { OffsetRange } from '../../../common/core/ranges/offsetRange.js'; import { IDiffEditorOptions } from '../../../common/config/editorOptions.js'; import { IRange } from '../../../common/core/range.js'; @@ -216,6 +217,10 @@ export class MultiDiffEditorWidgetImpl extends Disposable { )); this._contextKeyService.createKey(EditorContextKeys.inMultiDiffEditor.key, true); + this._register(bindContextKey(EditorContextKeys.diffEditorAutomaticRenderSideBySide, this._contextKeyService, reader => + this.activeControl.read(reader)?.renderSideBySideInAutomaticMode.read(reader) ?? true)); + this._register(bindContextKey(EditorContextKeys.diffEditorTemporaryInlineMode, this._contextKeyService, reader => + this.activeControl.read(reader)?.temporaryInlineMode.read(reader) ?? false)); this._lastDocStates = {}; @@ -393,6 +398,13 @@ export class MultiDiffEditorWidgetImpl extends Disposable { public getScopedInstantiationService(): IInstantiationService { return this._instantiationService; } + + public resetWidthBasedLayout(): void { + for (const item of this._viewItemsInfo.get().items) { + item.template.get()?.editor.resetWidthBasedLayout(); + } + } + public reveal(resource: IMultiDiffResourceId, options?: RevealOptions): void { const viewItems = this._viewItems.get(); const index = viewItems.findIndex( diff --git a/src/vs/editor/common/config/editorOptions.ts b/src/vs/editor/common/config/editorOptions.ts index 6e6f3605c559..0a283d7abc78 100644 --- a/src/vs/editor/common/config/editorOptions.ts +++ b/src/vs/editor/common/config/editorOptions.ts @@ -863,6 +863,8 @@ export interface IEditorOptions { */ export const MINIMAP_GUTTER_WIDTH = 8; +export type DiffEditorViewMode = 'inline' | 'sideBySide' | 'automatic'; + export interface IDiffEditorBaseOptions { /** * Allow the user to resize the diff editor split view. diff --git a/src/vs/editor/test/browser/widget/multiDiffEditorWidget.test.ts b/src/vs/editor/test/browser/widget/multiDiffEditorWidget.test.ts index 96f7eff3c301..94f7c3bdd43a 100644 --- a/src/vs/editor/test/browser/widget/multiDiffEditorWidget.test.ts +++ b/src/vs/editor/test/browser/widget/multiDiffEditorWidget.test.ts @@ -28,6 +28,7 @@ import { MultiDiffEditorWidget } from '../../../browser/widget/multiDiffEditor/m import { IWorkbenchUIElementFactory } from '../../../browser/widget/multiDiffEditor/workbenchUIElementFactory.js'; import { EditorOption } from '../../../common/config/editorOptions.js'; import { IDocumentDiff, IDocumentDiffProvider } from '../../../common/diff/documentDiffProvider.js'; +import { EditorContextKeys } from '../../../common/editorContextKeys.js'; import { instantiateTextModel } from '../../common/testTextModel.js'; import { TestDiffProviderFactoryService } from '../diff/testDiffProviderFactoryService.js'; import { createCodeEditorServices } from '../testCodeEditor.js'; @@ -238,7 +239,7 @@ suite('MultiDiffEditorWidget', () => { {} satisfies IWorkbenchUIElementFactory, { variant: MultiDiffEditorVariant.Standard }, ); - widget.setRenderSideBySide(true, { useInlineViewWhenSpaceIsLimited: true }); + widget.setViewMode('automatic'); widget.layout(new Dimension(800, 600)); const viewModel = widget.createViewModel(model); await waitForState(viewModel.items, items => items.length === 1); @@ -248,6 +249,7 @@ suite('MultiDiffEditorWidget', () => { try { const activeControl = widget.getActiveControl(); const renderSideBySideWhenNarrow = activeControl?.renderSideBySide; + const automaticLayoutWhenNarrow = widget.getContextKeyService().getContextKeyValue(EditorContextKeys.diffEditorAutomaticRenderSideBySide.key); widget.layout(new Dimension(1000, 600)); assert.deepStrictEqual({ configuredAccessibilitySupport: updateOptionsSpy.firstCall.args[0].accessibilitySupport, @@ -255,6 +257,8 @@ suite('MultiDiffEditorWidget', () => { configuredUseInlineViewWhenSpaceIsLimited: updateOptionsSpy.firstCall.args[0].useInlineViewWhenSpaceIsLimited, renderSideBySideWhenNarrow, renderSideBySideWhenWide: activeControl?.renderSideBySide, + automaticLayoutWhenNarrow, + automaticLayoutWhenWide: widget.getContextKeyService().getContextKeyValue(EditorContextKeys.diffEditorAutomaticRenderSideBySide.key), optionsAppliedBeforeModel: updateOptionsSpy.calledBefore(setDiffModelSpy), effectiveAccessibilitySupport: activeControl?.getModifiedEditor().getOption(EditorOption.accessibilitySupport), }, { @@ -263,6 +267,8 @@ suite('MultiDiffEditorWidget', () => { configuredUseInlineViewWhenSpaceIsLimited: true, renderSideBySideWhenNarrow: false, renderSideBySideWhenWide: true, + automaticLayoutWhenNarrow: false, + automaticLayoutWhenWide: true, optionsAppliedBeforeModel: true, effectiveAccessibilitySupport: AccessibilitySupport.Disabled, }); diff --git a/src/vs/sessions/browser/menus.ts b/src/vs/sessions/browser/menus.ts index 5cf11c63ce30..49de5a94e17d 100644 --- a/src/vs/sessions/browser/menus.ts +++ b/src/vs/sessions/browser/menus.ts @@ -60,6 +60,7 @@ export const Menus = { SessionsEditorHeaderPrimary: new MenuId('SessionsEditorHeaderPrimary'), SessionsEditorHeaderLayout: new MenuId('SessionsEditorHeaderLayout'), SessionsEditorTitle: new MenuId('SessionsEditorTitle'), + SessionsDiffEditorView: new MenuId('SessionsDiffEditorView'), SessionsEditorTabsBarContext: new MenuId('SessionsEditorTabsBarContext'), SessionsEditorTabsBarAddTab: new MenuId('SessionsEditorTabsBarAddTab'), /** diff --git a/src/vs/sessions/contrib/changes/browser/changesViewActions.ts b/src/vs/sessions/contrib/changes/browser/changesViewActions.ts index 6d1edb789dcb..e9b3bcf9d34a 100644 --- a/src/vs/sessions/contrib/changes/browser/changesViewActions.ts +++ b/src/vs/sessions/contrib/changes/browser/changesViewActions.ts @@ -8,6 +8,7 @@ import { Disposable } from '../../../../base/common/lifecycle.js'; import { observableFromEvent } from '../../../../base/common/observable.js'; import { isEqual } from '../../../../base/common/resources.js'; import { URI } from '../../../../base/common/uri.js'; +import { EditorContextKeys } from '../../../../editor/common/editorContextKeys.js'; import { localize, localize2 } from '../../../../nls.js'; import { Action2, IAction2Options, MenuId, MenuRegistry, registerAction2 } from '../../../../platform/actions/common/actions.js'; import { ICommandService } from '../../../../platform/commands/common/commands.js'; @@ -15,12 +16,13 @@ import { ContextKeyExpr, IContextKeyService } from '../../../../platform/context import { ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js'; import { bindContextKey } from '../../../../platform/observable/common/platformObservableUtils.js'; import { ITelemetryService } from '../../../../platform/telemetry/common/telemetry.js'; -import { TOGGLE_DIFF_SIDE_BY_SIDE } from '../../../../workbench/browser/parts/editor/diffEditorCommands.js'; +import { DIFF_VIEW_MODE_INLINE_TEMPORARY, SET_DIFF_VIEW_MODE_AUTOMATIC, SET_DIFF_VIEW_MODE_INLINE, SET_DIFF_VIEW_MODE_SIDE_BY_SIDE, TOGGLE_DIFF_SIDE_BY_SIDE } from '../../../../workbench/browser/parts/editor/diffEditorCommands.js'; import { IWorkbenchContribution, registerWorkbenchContribution2, WorkbenchPhase } from '../../../../workbench/common/contributions.js'; import { ActiveEditorContext, AuxiliaryBarVisibleContext, IsAuxiliaryWindowContext, IsSessionsWindowContext, IsTopRightEditorGroupContext, MainEditorAreaVisibleContext, TextCompareEditorActiveContext } from '../../../../workbench/common/contextkeys.js'; import { DiffEditorInput } from '../../../../workbench/common/editor/diffEditorInput.js'; import { IEditorService } from '../../../../workbench/services/editor/common/editorService.js'; import { IViewsService } from '../../../../workbench/services/views/common/viewsService.js'; +import { MultiDiffEditor } from '../../../../workbench/contrib/multiDiffEditor/browser/multiDiffEditor.js'; import { Menus } from '../../../browser/menus.js'; import { CustomViewVisibleContext, SessionHasChangesContext, SessionIsCreatedContext, SinglePaneDiffEditorInputActiveContext, SinglePaneLayoutEnabledContext } from '../../../common/contextkeys.js'; import { logChangesViewViewModeChange } from '../../../common/sessionsTelemetry.js'; @@ -28,7 +30,7 @@ import { ISessionsService } from '../../../services/sessions/browser/sessionsSer import { OPEN_PULL_REQUEST_ACTION_ID } from '../../github/common/types.js'; import { ActiveSessionContextKeys, CHANGES_VIEW_ID, ChangesContextKeys, ChangesViewMode, SESSIONS_CHANGES_OPEN_SINGLE_FILE_DIFF_SETTING } from '../common/changes.js'; import { IChangesViewService } from '../common/changesViewService.js'; -import { SessionsDiffRenderSideBySideContext } from '../../editor/common/diffEditorOptionsService.js'; +import { SessionsDiffViewModeContext } from '../../editor/common/diffEditorOptionsService.js'; import { CHANGES_HEADER_ACTIONS_ID } from './changesView.js'; import { SessionChangesEditor } from './sessionChangesEditor.js'; @@ -122,10 +124,9 @@ class OpenPullRequestAction extends Action2 { registerAction2(OpenPullRequestAction); -const singlePaneChangesEditorActive = ContextKeyExpr.and( +const agentsChangesEditorActive = ContextKeyExpr.and( IsSessionsWindowContext, - ActiveEditorContext.isEqualTo(SessionChangesEditor.ID), - SinglePaneLayoutEnabledContext + ActiveEditorContext.isEqualTo(SessionChangesEditor.ID) ); const singlePaneFileDiffEditorActive = ContextKeyExpr.and( @@ -134,12 +135,25 @@ const singlePaneFileDiffEditorActive = ContextKeyExpr.and( SinglePaneLayoutEnabledContext ); -const singlePaneTextDiffEditorActive = ContextKeyExpr.and( +const agentsTextDiffEditorActive = ContextKeyExpr.and( IsSessionsWindowContext, - TextCompareEditorActiveContext, - SinglePaneLayoutEnabledContext + TextCompareEditorActiveContext ); +const agentsMultiDiffEditorActive = ContextKeyExpr.and( + IsSessionsWindowContext, + ActiveEditorContext.isEqualTo(MultiDiffEditor.ID) +); + +const agentsDiffEditorActive = ContextKeyExpr.or( + agentsChangesEditorActive, + agentsTextDiffEditorActive, + agentsMultiDiffEditorActive +); + +const singlePaneChangesEditorActive = ContextKeyExpr.and(agentsChangesEditorActive, SinglePaneLayoutEnabledContext); +const singlePaneTextDiffEditorActive = ContextKeyExpr.and(agentsTextDiffEditorActive, SinglePaneLayoutEnabledContext); + // Title-bar (tab-row) gate that does NOT require the editor content area to be // visible, so session-level title actions (e.g. Create Pull Request) stay available // when the editor area is closed but the docked tab bar is still shown. @@ -166,8 +180,19 @@ const singlePaneTextDiffEditorTitle = ContextKeyExpr.and( IsTopRightEditorGroupContext ); +const singlePaneMultiDiffEditorTitle = ContextKeyExpr.and( + agentsMultiDiffEditorActive, + SinglePaneLayoutEnabledContext, + IsAuxiliaryWindowContext.toNegated(), + IsTopRightEditorGroupContext +); + const singlePaneDiffEditorTitleVisible = ContextKeyExpr.and( - ContextKeyExpr.or(singlePaneChangesEditorTitle, singlePaneTextDiffEditorTitle), + ContextKeyExpr.or( + singlePaneChangesEditorTitle, + singlePaneTextDiffEditorTitle, + singlePaneMultiDiffEditorTitle + ), MainEditorAreaVisibleContext ); @@ -323,19 +348,66 @@ registerAction2(ExpandAllSessionChangesDiffsAction); // user's keybinding for it carries over here (issue #324765). The sessions override of // IDiffEditorCommandsService updates the Changes editor's own preferred layout. -// The action changes the preferred layout. Side by side still falls back to inline -// when the editor is narrow, so the label must not promise an immediate layout. -MenuRegistry.appendMenuItem(Menus.SessionsEditorHeaderLayout, { +MenuRegistry.appendMenuItem(Menus.SessionsEditorTitle, { + submenu: Menus.SessionsDiffEditorView, + title: localize('diffView', "Diff View"), + group: '1_diff', + order: 10, + when: singlePaneDiffEditorTitleVisible, +}); +MenuRegistry.appendMenuItem(MenuId.EditorTitle, { + submenu: Menus.SessionsDiffEditorView, + title: localize('diffView', "Diff View"), + group: '1_diff', + order: 10, + when: ContextKeyExpr.and(agentsDiffEditorActive, SinglePaneLayoutEnabledContext.negate()), +}); +MenuRegistry.appendMenuItem(Menus.SessionsDiffEditorView, { command: { - id: TOGGLE_DIFF_SIDE_BY_SIDE, - title: localize('alwaysShowInlineDiff', "Always Show Inline Diff"), - tooltip: localize('alwaysShowInlineDiff.tooltip', "Always uses inline layout."), - icon: Codicon.diffSidebyside, - toggled: SessionsDiffRenderSideBySideContext.negate(), + id: SET_DIFF_VIEW_MODE_INLINE, + title: localize('diffView.inline', "Inline"), + toggled: SessionsDiffViewModeContext.isEqualTo('inline'), }, - group: 'secondary/1_diff', - order: 20, - when: singlePaneDiffEditorTitleVisible + group: '1_view', + order: 1, +}); +MenuRegistry.appendMenuItem(Menus.SessionsDiffEditorView, { + command: { + id: SET_DIFF_VIEW_MODE_SIDE_BY_SIDE, + title: localize('diffView.sideBySide', "Side by Side"), + toggled: SessionsDiffViewModeContext.isEqualTo('sideBySide'), + }, + group: '1_view', + order: 2, +}); +for (const [title, when] of [ + [localize('diffView.automaticSideBySide', "Automatic (Currently Side by Side)"), EditorContextKeys.diffEditorAutomaticRenderSideBySide], + [localize('diffView.automaticInline', "Automatic (Currently Inline)"), EditorContextKeys.diffEditorAutomaticRenderSideBySide.toNegated()], +] as const) { + MenuRegistry.appendMenuItem(Menus.SessionsDiffEditorView, { + command: { + id: SET_DIFF_VIEW_MODE_AUTOMATIC, + title, + toggled: ContextKeyExpr.and( + SessionsDiffViewModeContext.isEqualTo('automatic'), + EditorContextKeys.diffEditorTemporaryInlineMode.toNegated(), + ), + }, + group: '1_view', + order: 3, + when, + }); +} +MenuRegistry.appendMenuItem(Menus.SessionsDiffEditorView, { + command: { + id: DIFF_VIEW_MODE_INLINE_TEMPORARY, + title: localize('diffView.inlineTemporary', "Inline (Temporary)"), + toggled: EditorContextKeys.diffEditorTemporaryInlineMode, + precondition: ContextKeyExpr.false(), + }, + group: '1_view', + order: 4, + when: EditorContextKeys.diffEditorTemporaryInlineMode, }); // Discoverable in the command palette while a Changes diff editor is visible. @@ -345,7 +417,7 @@ MenuRegistry.appendMenuItem(MenuId.CommandPalette, { title: localize2('togglePreferredDiffView', "Toggle Preferred Diff View"), category: localize2('changes', "Changes"), }, - when: singlePaneDiffEditorTitleVisible + when: agentsDiffEditorActive }); class OpenChangesAction extends Action2 { diff --git a/src/vs/sessions/contrib/changes/browser/sessionChangesEditor.ts b/src/vs/sessions/contrib/changes/browser/sessionChangesEditor.ts index f6699a915968..ce4196da1258 100644 --- a/src/vs/sessions/contrib/changes/browser/sessionChangesEditor.ts +++ b/src/vs/sessions/contrib/changes/browser/sessionChangesEditor.ts @@ -178,6 +178,10 @@ export class SessionChangesEditor extends AbstractEditorWithViewState this._onDidChangeControl.fire())); this.widget.setPaddingBottom(CHANGES_LIST_BOTTOM_PADDING_PX); this._register(autorun(reader => { - this.widget?.setRenderSideBySide(this.diffEditorOptionsService.renderSideBySide.read(reader), { useInlineViewWhenSpaceIsLimited: true }); + this.widget?.setViewMode(this.diffEditorOptionsService.viewMode.read(reader)); })); } @@ -293,6 +298,14 @@ export class SessionChangesEditor extends AbstractEditorWithViewState') - : localize('sessionsChanges.diffView.classic', "File diffs can use side-by-side or inline layout. Use Inline View in the editor title area's More Actions menu, or use the Toggle Inline View command to switch the layout{0}.", '')); + ? localize('sessionsChanges.diffView.singlePane', "Use Diff View in the editor title area's More Actions menu to select inline, side-by-side, or automatic layout. The Toggle Preferred Diff View command switches between inline and automatic layout{0}.", '') + : localize('sessionsChanges.diffView.classic', "Use Diff View in the editor title area's More Actions menu to select inline, side-by-side, or automatic layout. The Toggle Preferred Diff View command switches between inline and automatic layout{0}.", '')); return new AccessibleContentProvider( AccessibleViewProviderId.SessionsChanges, diff --git a/src/vs/sessions/contrib/changes/test/browser/changesViewActions.test.ts b/src/vs/sessions/contrib/changes/test/browser/changesViewActions.test.ts index 454f6d49b384..0f79acc2f7f7 100644 --- a/src/vs/sessions/contrib/changes/test/browser/changesViewActions.test.ts +++ b/src/vs/sessions/contrib/changes/test/browser/changesViewActions.test.ts @@ -9,14 +9,13 @@ import { constObservable } from '../../../../../base/common/observable.js'; import { ThemeIcon } from '../../../../../base/common/themables.js'; import { mock } from '../../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; -import { isIMenuItem, MenuId, MenuRegistry } from '../../../../../platform/actions/common/actions.js'; -import { isICommandActionToggleInfo } from '../../../../../platform/action/common/action.js'; +import { isIMenuItem, isISubmenuItem, MenuId, MenuRegistry } from '../../../../../platform/actions/common/actions.js'; import { CommandsRegistry, ICommandService } from '../../../../../platform/commands/common/commands.js'; import { Context } from '../../../../../platform/contextkey/browser/contextKeyService.js'; import { ContextKeyExpression } from '../../../../../platform/contextkey/common/contextkey.js'; import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { EditorContextKeys } from '../../../../../editor/common/editorContextKeys.js'; -import { SessionsDiffRenderSideBySideContext } from '../../../editor/common/diffEditorOptionsService.js'; +import { SessionsDiffViewModeContext } from '../../../editor/common/diffEditorOptionsService.js'; import { ActiveEditorContext, AuxiliaryBarVisibleContext, IsAuxiliaryWindowContext, IsSessionsWindowContext, IsTopRightEditorGroupContext, MainEditorAreaVisibleContext, TextCompareEditorActiveContext } from '../../../../../workbench/common/contextkeys.js'; import { ChatPetAchievementId, ChatPetAchievementIds } from '../../../../../workbench/contrib/chat/browser/chatPetAchievements.js'; import { IChatPetService } from '../../../../../workbench/contrib/chat/browser/chatPetService.js'; @@ -28,6 +27,7 @@ import { IActiveSession } from '../../../../services/sessions/common/sessionsMan import { ChangesContextKeys, ChangesViewMode } from '../../common/changes.js'; import { CustomViewVisibleContext, IsPhoneLayoutContext, SessionHasChangesContext, SessionHasWorkspaceContext, SessionIsCreatedContext, SinglePaneDiffEditorInputActiveContext, SinglePaneLayoutEnabledContext } from '../../../../common/contextkeys.js'; import { SessionChangesEditor } from '../../browser/sessionChangesEditor.js'; +import { MultiDiffEditor } from '../../../../../workbench/contrib/multiDiffEditor/browser/multiDiffEditor.js'; import { CHANGES_HEADER_ACTIONS_ID, unlockChatPetCreatePullRequestAchievement } from '../../browser/changesView.js'; import { SessionsChangesAccessibilityHelp } from '../../browser/sessionsChangesAccessibilityHelp.js'; import '../../browser/changesViewActions.js'; @@ -189,58 +189,70 @@ suite('Changes View Actions', () => { }); }); - test('always show inline diff is contributed to the editor header layout overflow menu for multi-file and single-file diffs', () => { - const item = MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderLayout) - .filter(isIMenuItem) - .find(item => item.command.id === 'toggle.diff.renderSideBySide'); + test('Diff View submenu is contributed for text and multi-diff editors in both Agents layouts', () => { + const getSubmenu = (menuId: MenuId) => MenuRegistry.getMenuItems(menuId) + .filter(isISubmenuItem) + .find(item => item.submenu === Menus.SessionsDiffEditorView); + const singlePane = getSubmenu(Menus.SessionsEditorTitle); + const classic = getSubmenu(MenuId.EditorTitle); - assert.ok(item, 'expected the preferred diff view action in the editor header layout overflow menu'); - const when = item.when?.serialize() ?? ''; - const toggled = item.command.toggled; - const toggledCondition = isICommandActionToggleInfo(toggled) ? toggled.condition : toggled; - const nonTextDiffContext = new Context(1, null); - nonTextDiffContext.setValue(IsSessionsWindowContext.key, true); - nonTextDiffContext.setValue(SinglePaneDiffEditorInputActiveContext.key, true); - nonTextDiffContext.setValue(SinglePaneLayoutEnabledContext.key, true); - nonTextDiffContext.setValue(IsAuxiliaryWindowContext.key, false); - nonTextDiffContext.setValue(IsTopRightEditorGroupContext.key, true); - nonTextDiffContext.setValue(MainEditorAreaVisibleContext.key, true); - const toggleContext = new Context(1, null); - toggleContext.setValue(SessionsDiffRenderSideBySideContext.key, true); - const toggledWhenSideBySide = toggledCondition?.evaluate(toggleContext); - toggleContext.setValue(SessionsDiffRenderSideBySideContext.key, false); + assert.ok(singlePane); + assert.ok(classic); + const singlePaneWhen = singlePane.when?.serialize() ?? ''; + const classicWhen = classic.when?.serialize() ?? ''; assert.deepStrictEqual({ - id: item.command.id, - title: typeof item.command.title === 'string' ? item.command.title : item.command.title.value, - group: item.group, - order: item.order, - icon: ThemeIcon.isThemeIcon(item.command.icon) ? item.command.icon.id : undefined, - tooltip: typeof item.command.tooltip === 'string' ? item.command.tooltip : item.command.tooltip?.value, - hasStateSpecificTitle: isICommandActionToggleInfo(toggled), - toggledWhenSideBySide, - toggledWhenInline: toggledCondition?.evaluate(toggleContext), - hasSessionsWindowGate: when.includes(IsSessionsWindowContext.key), - hasActiveEditorGate: when.includes(ActiveEditorContext.key) && when.includes(SessionChangesEditor.ID), - hasTextCompareEditorGate: when.includes(TextCompareEditorActiveContext.key), - hasSinglePaneConfigGate: when.includes(SinglePaneLayoutEnabledContext.key), - hasEditorAreaVisibleGate: when.includes(MainEditorAreaVisibleContext.key), - matchesNonTextDiffContext: item.when?.evaluate(nonTextDiffContext) ?? false, + singlePaneTitle: typeof singlePane.title === 'string' ? singlePane.title : singlePane.title.value, + singlePaneGroup: singlePane.group, + singlePaneHasTextDiffGate: singlePaneWhen.includes(TextCompareEditorActiveContext.key), + singlePaneHasChangesGate: singlePaneWhen.includes(SessionChangesEditor.ID), + singlePaneHasMultiDiffGate: singlePaneWhen.includes(MultiDiffEditor.ID), + singlePaneHasLayoutGate: singlePaneWhen.includes(SinglePaneLayoutEnabledContext.key), + classicTitle: typeof classic.title === 'string' ? classic.title : classic.title.value, + classicHasSessionsGate: classicWhen.includes(IsSessionsWindowContext.key), + classicHasTextDiffGate: classicWhen.includes(TextCompareEditorActiveContext.key), + classicHasChangesGate: classicWhen.includes(SessionChangesEditor.ID), + classicHasMultiDiffGate: classicWhen.includes(MultiDiffEditor.ID), + classicHasLayoutGate: classicWhen.includes(SinglePaneLayoutEnabledContext.key), }, { - id: 'toggle.diff.renderSideBySide', - title: 'Always Show Inline Diff', - group: 'secondary/1_diff', - order: 20, - icon: Codicon.diffSidebyside.id, - tooltip: 'Always uses inline layout.', - hasStateSpecificTitle: false, - toggledWhenSideBySide: false, - toggledWhenInline: true, - hasSessionsWindowGate: true, - hasActiveEditorGate: true, - hasTextCompareEditorGate: true, - hasSinglePaneConfigGate: true, - hasEditorAreaVisibleGate: true, - matchesNonTextDiffContext: false, + singlePaneTitle: 'Diff View', + singlePaneGroup: '1_diff', + singlePaneHasTextDiffGate: true, + singlePaneHasChangesGate: true, + singlePaneHasMultiDiffGate: true, + singlePaneHasLayoutGate: true, + classicTitle: 'Diff View', + classicHasSessionsGate: true, + classicHasTextDiffGate: true, + classicHasChangesGate: true, + classicHasMultiDiffGate: true, + classicHasLayoutGate: true, + }); + }); + + test('Agents Diff View submenu exposes inline, side-by-side, and automatic modes', () => { + const items = MenuRegistry.getMenuItems(Menus.SessionsDiffEditorView) + .filter(isIMenuItem) + .map(item => ({ + id: item.command.id, + title: typeof item.command.title === 'string' ? item.command.title : item.command.title.value, + group: item.group, + order: item.order, + })); + const modeContext = new Context(1, null); + modeContext.setValue(SessionsDiffViewModeContext.key, 'sideBySide'); + + assert.deepStrictEqual({ + items, + selectedMode: modeContext.getValue(SessionsDiffViewModeContext.key), + }, { + items: [ + { id: 'diffEditor.setViewMode.inline', title: 'Inline', group: '1_view', order: 1 }, + { id: 'diffEditor.setViewMode.sideBySide', title: 'Side by Side', group: '1_view', order: 2 }, + { id: 'diffEditor.setViewMode.automatic', title: 'Automatic (Currently Side by Side)', group: '1_view', order: 3 }, + { id: 'diffEditor.setViewMode.automatic', title: 'Automatic (Currently Inline)', group: '1_view', order: 3 }, + { id: 'diffEditor.viewMode.inlineTemporary', title: 'Inline (Temporary)', group: '1_view', order: 4 }, + ], + selectedMode: 'sideBySide', }); }); @@ -258,8 +270,7 @@ suite('Changes View Actions', () => { hasSessionsWindowGate: when.includes(IsSessionsWindowContext.key), hasActiveEditorGate: when.includes(ActiveEditorContext.key) && when.includes(SessionChangesEditor.ID), hasTextCompareEditorGate: when.includes(TextCompareEditorActiveContext.key), - hasSinglePaneConfigGate: when.includes(SinglePaneLayoutEnabledContext.key), - hasEditorAreaVisibleGate: when.includes(MainEditorAreaVisibleContext.key), + hasMultiDiffEditorGate: when.includes(MultiDiffEditor.ID), }, { id: 'toggle.diff.renderSideBySide', title: 'Toggle Preferred Diff View', @@ -267,8 +278,7 @@ suite('Changes View Actions', () => { hasSessionsWindowGate: true, hasActiveEditorGate: true, hasTextCompareEditorGate: true, - hasSinglePaneConfigGate: true, - hasEditorAreaVisibleGate: true, + hasMultiDiffEditorGate: true, }); }); @@ -286,11 +296,11 @@ suite('Changes View Actions', () => { } test('Changes accessibility help describes the single-pane diff action', () => { - assert.strictEqual(getChangesAccessibilityHelp(true).includes('Use Always Show Inline Diff in the editor header\'s More Actions menu'), true); + assert.strictEqual(getChangesAccessibilityHelp(true).includes('Use Diff View in the editor title area\'s More Actions menu'), true); }); test('Changes accessibility help describes the classic diff action', () => { - assert.strictEqual(getChangesAccessibilityHelp(false).includes('Use Inline View in the editor title area\'s More Actions menu'), true); + assert.strictEqual(getChangesAccessibilityHelp(false).includes('Use Diff View in the editor title area\'s More Actions menu'), true); }); test('view mode toggles are moved to the editor header layout overflow for non-text single-file diffs', () => { diff --git a/src/vs/sessions/contrib/editor/browser/diffEditor.sessions.contribution.ts b/src/vs/sessions/contrib/editor/browser/diffEditor.sessions.contribution.ts index 79c7e05be4ce..044697d9a8bf 100644 --- a/src/vs/sessions/contrib/editor/browser/diffEditor.sessions.contribution.ts +++ b/src/vs/sessions/contrib/editor/browser/diffEditor.sessions.contribution.ts @@ -8,6 +8,7 @@ import { isEqual } from '../../../../base/common/resources.js'; import { Disposable } from '../../../../base/common/lifecycle.js'; import { autorun } from '../../../../base/common/observable.js'; import { isDiffEditor } from '../../../../editor/browser/editorBrowser.js'; +import { DiffEditorViewMode } from '../../../../editor/common/config/editorOptions.js'; import { ITextResourceConfigurationService } from '../../../../editor/common/services/textResourceConfiguration.js'; import { IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; import { InstantiationType, registerSingleton } from '../../../../platform/instantiation/common/extensions.js'; @@ -15,6 +16,7 @@ import { DiffEditorCommandsService, IDiffEditorCommandsService } from '../../../ import { TextDiffEditor } from '../../../../workbench/browser/parts/editor/textDiffEditor.js'; import { IWorkbenchContribution, registerWorkbenchContribution2, WorkbenchPhase } from '../../../../workbench/common/contributions.js'; import { IEditorService } from '../../../../workbench/services/editor/common/editorService.js'; +import { MultiDiffEditor } from '../../../../workbench/contrib/multiDiffEditor/browser/multiDiffEditor.js'; import { SessionChangesEditor } from '../../changes/browser/sessionChangesEditor.js'; import { IDiffEditorOptionsService } from '../common/diffEditorOptionsService.js'; import { DiffEditorOptionsService } from './diffEditorOptionsService.js'; @@ -33,8 +35,12 @@ export class SessionsDiffEditorCommandsService extends DiffEditorCommandsService override async toggleRenderSideBySide(args: unknown[]): Promise { const resource = args[0] instanceof URI ? args[0] : undefined; - if (resource || !(this.editorService.activeEditorPane instanceof SessionChangesEditor)) { + if (resource || !(this.editorService.activeEditorPane instanceof SessionChangesEditor || this.editorService.activeEditorPane instanceof MultiDiffEditor)) { for (const pane of [this.editorService.activeEditorPane, ...this.editorService.visibleEditorPanes]) { + if (pane instanceof MultiDiffEditor) { + this.diffEditorOptionsService.toggleRenderSideBySide(); + return; + } if (!(pane instanceof TextDiffEditor)) { continue; } @@ -54,7 +60,7 @@ export class SessionsDiffEditorCommandsService extends DiffEditorCommandsService } } - if (this.editorService.activeEditorPane instanceof SessionChangesEditor) { + if (this.editorService.activeEditorPane instanceof SessionChangesEditor || this.editorService.activeEditorPane instanceof MultiDiffEditor) { this.diffEditorOptionsService.toggleRenderSideBySide(); return; } @@ -66,6 +72,38 @@ export class SessionsDiffEditorCommandsService extends DiffEditorCommandsService return super.toggleRenderSideBySide(args); } + + override async setViewMode(args: unknown[], mode: DiffEditorViewMode): Promise { + const resource = args[0] instanceof URI ? args[0] : undefined; + const activeEditorPane = this.editorService.activeEditorPane; + if (activeEditorPane instanceof SessionChangesEditor || activeEditorPane instanceof MultiDiffEditor) { + this.diffEditorOptionsService.setViewMode(mode); + if (mode === 'automatic') { + activeEditorPane.resetDiffEditorWidthBasedLayout(); + } + return; + } + + for (const pane of [this.editorService.activeEditorPane, ...this.editorService.visibleEditorPanes]) { + if (!(pane instanceof TextDiffEditor)) { + continue; + } + const control = pane.getControl(); + if (!isDiffEditor(control)) { + continue; + } + const modifiedResource = control.getModifiedEditor().getModel()?.uri; + if (!resource || modifiedResource && isEqual(resource, modifiedResource)) { + this.diffEditorOptionsService.setViewMode(mode); + if (mode === 'automatic') { + control.resetWidthBasedLayout(); + } + return; + } + } + + return super.setViewMode(args, mode); + } } export class SessionsDiffEditorLayoutContribution extends Disposable implements IWorkbenchContribution { @@ -80,19 +118,24 @@ export class SessionsDiffEditorLayoutContribution extends Disposable implements this._register(this.editorService.onDidActiveEditorChange(() => this.applyLayout())); this._register(this.editorService.onDidVisibleEditorsChange(() => this.applyLayout())); this._register(autorun(reader => { - this.diffEditorOptionsService.renderSideBySide.read(reader); + this.diffEditorOptionsService.viewMode.read(reader); this.applyLayout(); })); } private applyLayout(): void { - const renderSideBySide = this.diffEditorOptionsService.renderSideBySide.get(); + const viewMode = this.diffEditorOptionsService.viewMode.get(); for (const pane of new Set([this.editorService.activeEditorPane, ...this.editorService.visibleEditorPanes])) { if (pane instanceof TextDiffEditor) { const control = pane.getControl(); if (isDiffEditor(control)) { - control.updateOptions({ renderSideBySide, useInlineViewWhenSpaceIsLimited: true }); + control.updateOptions({ + renderSideBySide: viewMode !== 'inline', + useInlineViewWhenSpaceIsLimited: viewMode === 'automatic', + }); } + } else if (pane instanceof MultiDiffEditor) { + pane.setDiffEditorViewMode(viewMode); } } } diff --git a/src/vs/sessions/contrib/editor/browser/diffEditorOptionsService.ts b/src/vs/sessions/contrib/editor/browser/diffEditorOptionsService.ts index 024d24cd0b59..2826ed45c659 100644 --- a/src/vs/sessions/contrib/editor/browser/diffEditorOptionsService.ts +++ b/src/vs/sessions/contrib/editor/browser/diffEditorOptionsService.ts @@ -5,17 +5,20 @@ import { Disposable } from '../../../../base/common/lifecycle.js'; import { observableValue } from '../../../../base/common/observable.js'; +import { DiffEditorViewMode } from '../../../../editor/common/config/editorOptions.js'; import { IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; import { bindContextKey } from '../../../../platform/observable/common/platformObservableUtils.js'; import { IStorageService, StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js'; -import { IDiffEditorOptionsService, SessionsDiffRenderSideBySideContext } from '../common/diffEditorOptionsService.js'; +import { IDiffEditorOptionsService, SessionsDiffViewModeContext } from '../common/diffEditorOptionsService.js'; -const PREFERRED_RENDER_SIDE_BY_SIDE_STORAGE_KEY = 'sessions.diffEditor.renderSideBySide'; +const VIEW_MODE_STORAGE_KEY = 'sessions.diffEditor.viewMode'; +const LEGACY_RENDER_SIDE_BY_SIDE_STORAGE_KEY = 'sessions.diffEditor.renderSideBySide'; export class DiffEditorOptionsService extends Disposable implements IDiffEditorOptionsService { declare readonly _serviceBrand: undefined; + readonly viewMode; readonly renderSideBySide; constructor( @@ -23,13 +26,25 @@ export class DiffEditorOptionsService extends Disposable implements IDiffEditorO @IContextKeyService contextKeyService: IContextKeyService, ) { super(); - this.renderSideBySide = observableValue(this, storageService.getBoolean(PREFERRED_RENDER_SIDE_BY_SIDE_STORAGE_KEY, StorageScope.PROFILE, true)); - this._register(bindContextKey(SessionsDiffRenderSideBySideContext, contextKeyService, reader => this.renderSideBySide.read(reader))); + const storedViewMode = storageService.get(VIEW_MODE_STORAGE_KEY, StorageScope.PROFILE); + const legacyRenderSideBySide = storageService.getBoolean(LEGACY_RENDER_SIDE_BY_SIDE_STORAGE_KEY, StorageScope.PROFILE); + this.viewMode = observableValue(this, isDiffEditorViewMode(storedViewMode) + ? storedViewMode + : legacyRenderSideBySide === false ? 'inline' : 'automatic'); + this.renderSideBySide = this.viewMode.map(this, mode => mode !== 'inline'); + this._register(bindContextKey(SessionsDiffViewModeContext, contextKeyService, reader => this.viewMode.read(reader))); + } + + setViewMode(mode: DiffEditorViewMode): void { + this.viewMode.set(mode, undefined); + this.storageService.store(VIEW_MODE_STORAGE_KEY, mode, StorageScope.PROFILE, StorageTarget.USER); } toggleRenderSideBySide(): void { - const renderSideBySide = !this.renderSideBySide.get(); - this.renderSideBySide.set(renderSideBySide, undefined); - this.storageService.store(PREFERRED_RENDER_SIDE_BY_SIDE_STORAGE_KEY, renderSideBySide, StorageScope.PROFILE, StorageTarget.USER); + this.setViewMode(this.viewMode.get() === 'inline' ? 'automatic' : 'inline'); } } + +function isDiffEditorViewMode(value: string | undefined): value is DiffEditorViewMode { + return value === 'inline' || value === 'sideBySide' || value === 'automatic'; +} diff --git a/src/vs/sessions/contrib/editor/common/diffEditorOptionsService.ts b/src/vs/sessions/contrib/editor/common/diffEditorOptionsService.ts index 1f1c1120f7cd..ed48da8f2f0f 100644 --- a/src/vs/sessions/contrib/editor/common/diffEditorOptionsService.ts +++ b/src/vs/sessions/contrib/editor/common/diffEditorOptionsService.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { IObservable } from '../../../../base/common/observable.js'; +import { DiffEditorViewMode } from '../../../../editor/common/config/editorOptions.js'; import { localize } from '../../../../nls.js'; import { RawContextKey } from '../../../../platform/contextkey/common/contextkey.js'; import { createDecorator } from '../../../../platform/instantiation/common/instantiation.js'; @@ -12,8 +13,10 @@ export const IDiffEditorOptionsService = createDecorator; readonly renderSideBySide: IObservable; + setViewMode(mode: DiffEditorViewMode): void; toggleRenderSideBySide(): void; } -export const SessionsDiffRenderSideBySideContext = new RawContextKey('sessionsDiffRenderSideBySide', true, localize('sessionsDiffRenderSideBySide', "Whether Agents window diffs prefer side-by-side layout")); +export const SessionsDiffViewModeContext = new RawContextKey('sessionsDiffViewMode', 'automatic', localize('sessionsDiffViewMode', "The preferred layout mode for diffs in the Agents window")); diff --git a/src/vs/sessions/contrib/editor/test/browser/diffEditor.sessions.contribution.test.ts b/src/vs/sessions/contrib/editor/test/browser/diffEditor.sessions.contribution.test.ts index 526adffd2954..d7804583bd0a 100644 --- a/src/vs/sessions/contrib/editor/test/browser/diffEditor.sessions.contribution.test.ts +++ b/src/vs/sessions/contrib/editor/test/browser/diffEditor.sessions.contribution.test.ts @@ -16,7 +16,7 @@ import { IEditorPane, IVisibleEditorPane } from '../../../../../workbench/common import { SessionChangesEditor } from '../../../changes/browser/sessionChangesEditor.js'; import { SessionsDiffEditorCommandsService, SessionsDiffEditorLayoutContribution } from '../../browser/diffEditor.sessions.contribution.js'; import { TextDiffEditor } from '../../../../../workbench/browser/parts/editor/textDiffEditor.js'; -import { IDiffEditorOptions } from '../../../../../editor/common/config/editorOptions.js'; +import { DiffEditorViewMode, IDiffEditorOptions } from '../../../../../editor/common/config/editorOptions.js'; import { ICodeEditor, IDiffEditor } from '../../../../../editor/browser/editorBrowser.js'; import { EditorType } from '../../../../../editor/common/editorCommon.js'; import { IDiffEditorOptionsService } from '../../common/diffEditorOptionsService.js'; @@ -25,7 +25,7 @@ suite('SessionsDiffEditorCommandsService', () => { const disposables = ensureNoDisposablesAreLeakedInTestSuite(); - function createService(activeEditorPane: IEditorPane | undefined, visibleEditorPanes: readonly IVisibleEditorPane[] = []): { service: SessionsDiffEditorCommandsService; getToggleCount(): number } { + function createService(activeEditorPane: IEditorPane | undefined, visibleEditorPanes: readonly IVisibleEditorPane[] = []): { service: SessionsDiffEditorCommandsService; getToggleCount(): number; setModes: DiffEditorViewMode[] } { const editorService = new class extends mock() { override get activeEditorPane() { return activeEditorPane as IVisibleEditorPane | undefined; } override get activeEditor() { return undefined; } @@ -37,12 +37,14 @@ suite('SessionsDiffEditorCommandsService', () => { override getContextKeyValue(): T | undefined { return undefined; } }; let toggleCount = 0; + const setModes: DiffEditorViewMode[] = []; const diffEditorOptionsService = new class extends mock() { override toggleRenderSideBySide(): void { toggleCount++; } + override setViewMode(mode: DiffEditorViewMode): void { setModes.push(mode); } }; const service = new SessionsDiffEditorCommandsService(editorService, textResourceConfigurationService, contextKeyService, diffEditorOptionsService); - return { service, getToggleCount: () => toggleCount }; + return { service, getToggleCount: () => toggleCount, setModes }; } function createTextDiffEditor(resource: URI, renderSideBySide: boolean, controlUpdates: IDiffEditorOptions[]): TextDiffEditor { @@ -72,6 +74,15 @@ suite('SessionsDiffEditorCommandsService', () => { assert.strictEqual(getToggleCount(), 1); }); + test('sets an explicit shared view mode from the Changes editor', async () => { + const changesEditor = Object.create(SessionChangesEditor.prototype) as IEditorPane; + const { service, setModes } = createService(changesEditor); + + await service.setViewMode([], 'sideBySide'); + + assert.deepStrictEqual(setModes, ['sideBySide']); + }); + test('toggles the shared preference when a narrow single-file diff is effectively inline', async () => { const resource = URI.file('/workspace/file.ts'); const controlUpdates: IDiffEditorOptions[] = []; @@ -123,7 +134,7 @@ suite('SessionsDiffEditorCommandsService', () => { }); }); - test('applies the shared responsive preference to all visible text diffs', () => { + test('applies the shared view mode to all visible text diffs', () => { const activeControlUpdates: IDiffEditorOptions[] = []; const visibleControlUpdates: IDiffEditorOptions[] = []; const activeEditor = createTextDiffEditor(URI.file('/workspace/active.ts'), false, activeControlUpdates); @@ -134,13 +145,15 @@ suite('SessionsDiffEditorCommandsService', () => { override get activeEditorPane() { return activeEditor as IVisibleEditorPane; } override get visibleEditorPanes() { return [visibleEditor as IVisibleEditorPane]; } }; - const renderSideBySide = observableValue('test', true); + const viewMode = observableValue('test', 'automatic'); const diffEditorOptionsService = new class extends mock() { - override readonly renderSideBySide = renderSideBySide; + override readonly viewMode = viewMode; + override readonly renderSideBySide = viewMode.map(this, mode => mode !== 'inline'); }; disposables.add(new SessionsDiffEditorLayoutContribution(editorService, diffEditorOptionsService)); - renderSideBySide.set(false, undefined); + viewMode.set('sideBySide', undefined); + viewMode.set('inline', undefined); assert.deepStrictEqual({ activeControlUpdates, @@ -148,11 +161,13 @@ suite('SessionsDiffEditorCommandsService', () => { }, { activeControlUpdates: [ { renderSideBySide: true, useInlineViewWhenSpaceIsLimited: true }, - { renderSideBySide: false, useInlineViewWhenSpaceIsLimited: true }, + { renderSideBySide: true, useInlineViewWhenSpaceIsLimited: false }, + { renderSideBySide: false, useInlineViewWhenSpaceIsLimited: false }, ], visibleControlUpdates: [ { renderSideBySide: true, useInlineViewWhenSpaceIsLimited: true }, - { renderSideBySide: false, useInlineViewWhenSpaceIsLimited: true }, + { renderSideBySide: true, useInlineViewWhenSpaceIsLimited: false }, + { renderSideBySide: false, useInlineViewWhenSpaceIsLimited: false }, ], }); }); diff --git a/src/vs/sessions/contrib/editor/test/browser/diffEditorOptionsService.test.ts b/src/vs/sessions/contrib/editor/test/browser/diffEditorOptionsService.test.ts index 86db04f84c08..9067679d8a8c 100644 --- a/src/vs/sessions/contrib/editor/test/browser/diffEditorOptionsService.test.ts +++ b/src/vs/sessions/contrib/editor/test/browser/diffEditorOptionsService.test.ts @@ -6,40 +6,64 @@ import assert from 'assert'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; import { MockContextKeyService } from '../../../../../platform/keybinding/test/common/mockKeybindingService.js'; -import { InMemoryStorageService, StorageScope } from '../../../../../platform/storage/common/storage.js'; -import { SessionsDiffRenderSideBySideContext } from '../../common/diffEditorOptionsService.js'; +import { InMemoryStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; +import { SessionsDiffViewModeContext } from '../../common/diffEditorOptionsService.js'; import { DiffEditorOptionsService } from '../../browser/diffEditorOptionsService.js'; suite('DiffEditorOptionsService', () => { const disposables = ensureNoDisposablesAreLeakedInTestSuite(); - test('defaults to responsive side by side and persists the shared preference', () => { + test('defaults to automatic and persists explicit modes', () => { const storageService = disposables.add(new InMemoryStorageService()); const contextKeyService = disposables.add(new MockContextKeyService()); const service = disposables.add(new DiffEditorOptionsService(storageService, contextKeyService)); const initial = { + viewMode: service.viewMode.get(), renderSideBySide: service.renderSideBySide.get(), - contextValue: contextKeyService.getContextKeyValue(SessionsDiffRenderSideBySideContext.key), - storedValue: storageService.getBoolean('sessions.diffEditor.renderSideBySide', StorageScope.PROFILE), + contextValue: contextKeyService.getContextKeyValue(SessionsDiffViewModeContext.key), + storedValue: storageService.get('sessions.diffEditor.viewMode', StorageScope.PROFILE), }; - service.toggleRenderSideBySide(); + service.setViewMode('sideBySide'); assert.deepStrictEqual({ initial, + viewMode: service.viewMode.get(), renderSideBySide: service.renderSideBySide.get(), - contextValue: contextKeyService.getContextKeyValue(SessionsDiffRenderSideBySideContext.key), - storedValue: storageService.getBoolean('sessions.diffEditor.renderSideBySide', StorageScope.PROFILE), + contextValue: contextKeyService.getContextKeyValue(SessionsDiffViewModeContext.key), + storedValue: storageService.get('sessions.diffEditor.viewMode', StorageScope.PROFILE), }, { initial: { + viewMode: 'automatic', renderSideBySide: true, - contextValue: true, + contextValue: 'automatic', storedValue: undefined, }, - renderSideBySide: false, - contextValue: false, - storedValue: false, + viewMode: 'sideBySide', + renderSideBySide: true, + contextValue: 'sideBySide', + storedValue: 'sideBySide', + }); + }); + + test('migrates the legacy inline preference and toggles back to automatic', () => { + const storageService = disposables.add(new InMemoryStorageService()); + storageService.store('sessions.diffEditor.renderSideBySide', false, StorageScope.PROFILE, StorageTarget.USER); + const contextKeyService = disposables.add(new MockContextKeyService()); + const service = disposables.add(new DiffEditorOptionsService(storageService, contextKeyService)); + + const migratedViewMode = service.viewMode.get(); + service.toggleRenderSideBySide(); + + assert.deepStrictEqual({ + migratedViewMode, + viewMode: service.viewMode.get(), + storedValue: storageService.get('sessions.diffEditor.viewMode', StorageScope.PROFILE), + }, { + migratedViewMode: 'inline', + viewMode: 'automatic', + storedValue: 'automatic', }); }); }); diff --git a/src/vs/workbench/browser/parts/editor/diffEditorCommands.ts b/src/vs/workbench/browser/parts/editor/diffEditorCommands.ts index 4bb1e32c7783..78dd013b9805 100644 --- a/src/vs/workbench/browser/parts/editor/diffEditorCommands.ts +++ b/src/vs/workbench/browser/parts/editor/diffEditorCommands.ts @@ -10,7 +10,8 @@ import { ContextKeyExpr } from '../../../../platform/contextkey/common/contextke import { KeybindingsRegistry, KeybindingWeight } from '../../../../platform/keybinding/common/keybindingsRegistry.js'; import { ActiveCompareEditorCanSwapContext, ActiveCustomEditorDiffCanToggleLayoutContext, TextCompareEditorActiveContext, TextCompareEditorVisibleContext } from '../../../common/contextkeys.js'; import { EditorContextKeys } from '../../../../editor/common/editorContextKeys.js'; -import { DiffEditorViewMode, FocusTextDiffEditorMode, IDiffEditorCommandsService } from './diffEditorCommandsService.js'; +import { DiffEditorViewMode } from '../../../../editor/common/config/editorOptions.js'; +import { FocusTextDiffEditorMode, IDiffEditorCommandsService } from './diffEditorCommandsService.js'; export const TOGGLE_DIFF_SIDE_BY_SIDE = 'toggle.diff.renderSideBySide'; export const SET_DIFF_VIEW_MODE_INLINE = 'diffEditor.setViewMode.inline'; diff --git a/src/vs/workbench/browser/parts/editor/diffEditorCommandsService.ts b/src/vs/workbench/browser/parts/editor/diffEditorCommandsService.ts index 95a8df6a667f..7f01c6c855b4 100644 --- a/src/vs/workbench/browser/parts/editor/diffEditorCommandsService.ts +++ b/src/vs/workbench/browser/parts/editor/diffEditorCommandsService.ts @@ -5,17 +5,26 @@ import { isEqual } from '../../../../base/common/resources.js'; import { URI } from '../../../../base/common/uri.js'; -import { isDiffEditor } from '../../../../editor/browser/editorBrowser.js'; +import { IDiffEditor, isDiffEditor } from '../../../../editor/browser/editorBrowser.js'; import { ITextResourceConfigurationService } from '../../../../editor/common/services/textResourceConfiguration.js'; +import { DiffEditorViewMode } from '../../../../editor/common/config/editorOptions.js'; import { createDecorator } from '../../../../platform/instantiation/common/instantiation.js'; import { IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; import { ActiveCustomEditorDiffCanToggleLayoutContext } from '../../../common/contextkeys.js'; import { DiffEditorInput } from '../../../common/editor/diffEditorInput.js'; import { EditorInput } from '../../../common/editor/editorInput.js'; import { IEditorService } from '../../../services/editor/common/editorService.js'; -import { EditorResourceAccessor, isDiffEditorInput, IUntypedEditorInput, SideBySideEditor } from '../../../common/editor.js'; +import { EditorResourceAccessor, IEditorPane, isDiffEditorInput, IUntypedEditorInput, SideBySideEditor } from '../../../common/editor.js'; import { TextDiffEditor } from './textDiffEditor.js'; +interface IDiffEditorWidthBasedLayoutReset { + resetDiffEditorWidthBasedLayout(): void; +} + +function hasDiffEditorWidthBasedLayoutReset(pane: IEditorPane): pane is IEditorPane & IDiffEditorWidthBasedLayoutReset { + return 'resetDiffEditorWidthBasedLayout' in pane && typeof pane.resetDiffEditorWidthBasedLayout === 'function'; +} + export const IDiffEditorCommandsService = createDecorator('diffEditorCommandsService'); /** Which side of the active diff editor to focus. */ @@ -25,8 +34,6 @@ export const enum FocusTextDiffEditorMode { Toggle } -export type DiffEditorViewMode = 'inline' | 'sideBySide' | 'automatic'; - /** * Backs the diff-editor commands (see {@link registerDiffEditorCommands}). The Agents window * overrides this to also drive its multi-diff Changes editor. @@ -78,8 +85,8 @@ export class DiffEditorCommandsService implements IDiffEditorCommandsService { } async setViewMode(args: unknown[], mode: DiffEditorViewMode): Promise { - const activeTextDiffEditor = this.getActiveTextDiffEditor(args); - const control = activeTextDiffEditor?.getControl(); + const activeDiffEditor = this.getActiveDiffEditor(args); + const control = activeDiffEditor?.control; const modifiedResource = control?.getModifiedEditor().getModel()?.uri; if (!modifiedResource) { return; @@ -100,7 +107,11 @@ export class DiffEditorCommandsService implements IDiffEditorCommandsService { this.textResourceConfigurationService.updateValue(modifiedResource, 'diffEditor.renderSideBySide', true), this.textResourceConfigurationService.updateValue(modifiedResource, 'diffEditor.useInlineViewWhenSpaceIsLimited', true), ]); - control.resetWidthBasedLayout(); + if (activeDiffEditor && hasDiffEditorWidthBasedLayoutReset(activeDiffEditor.pane)) { + activeDiffEditor.pane.resetDiffEditorWidthBasedLayout(); + } else { + control.resetWidthBasedLayout(); + } break; } } @@ -222,9 +233,34 @@ export class DiffEditorCommandsService implements IDiffEditorCommandsService { return undefined; } + private getActiveDiffEditor(args: unknown[]): { pane: IEditorPane; control: IDiffEditor } | undefined { + const textDiffEditor = this.getActiveTextDiffEditor(args); + const textDiffControl = textDiffEditor?.getControl(); + if (textDiffEditor && textDiffControl) { + return { pane: textDiffEditor, control: textDiffControl }; + } + + const resource = args.length > 0 && args[0] instanceof URI ? args[0] : undefined; + for (const pane of [this.editorService.activeEditorPane, ...this.editorService.visibleEditorPanes]) { + const control = pane?.getControl(); + if (!pane || !isDiffEditor(control)) { + continue; + } + + const modifiedResource = control.getModifiedEditor().getModel()?.uri; + const inputResource = pane.input + ? EditorResourceAccessor.getCanonicalUri(pane.input, { supportSideBySide: SideBySideEditor.PRIMARY }) + : undefined; + if (!resource || (modifiedResource && isEqual(modifiedResource, resource)) || (inputResource && isEqual(inputResource, resource))) { + return { pane, control }; + } + } + + return undefined; + } + private getActiveDiffModifiedResource(args: unknown[]): URI | undefined { - const activeTextDiffEditor = this.getActiveTextDiffEditor(args); - const model = activeTextDiffEditor?.getControl()?.getModifiedEditor()?.getModel(); + const model = this.getActiveDiffEditor(args)?.control.getModifiedEditor().getModel(); if (model) { return model.uri; } diff --git a/src/vs/workbench/browser/parts/editor/editor.contribution.ts b/src/vs/workbench/browser/parts/editor/editor.contribution.ts index 29c74f36fd15..5f4cad341b60 100644 --- a/src/vs/workbench/browser/parts/editor/editor.contribution.ts +++ b/src/vs/workbench/browser/parts/editor/editor.contribution.ts @@ -425,7 +425,7 @@ MenuRegistry.appendMenuItem(MenuId.EditorTitle, { title: localize('diffView', "Diff View"), group: '1_diff', order: 10, - when: ContextKeyExpr.has('isInDiffEditor'), + when: ContextKeyExpr.and(ContextKeyExpr.has('isInDiffEditor'), IsSessionsWindowContext.toNegated()), }); MenuRegistry.appendMenuItem(MenuId.EditorTitle, { command: { id: TOGGLE_DIFF_SIDE_BY_SIDE, title: localize('inlineView', "Inline View"), toggled: ContextKeyExpr.equals('config.diffEditor.renderSideBySide', false) }, diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/actions.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/actions.ts index 413bd9e3d7ac..1028157ff5c9 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/actions.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/actions.ts @@ -11,8 +11,8 @@ import { Selection } from '../../../../editor/common/core/selection.js'; import { EditorContextKeys } from '../../../../editor/common/editorContextKeys.js'; import { ILanguageService } from '../../../../editor/common/languages/language.js'; import { IModelService } from '../../../../editor/common/services/model.js'; -import { localize2 } from '../../../../nls.js'; -import { Action2, MenuId } from '../../../../platform/actions/common/actions.js'; +import { localize, localize2 } from '../../../../nls.js'; +import { Action2, MenuId, MenuRegistry } from '../../../../platform/actions/common/actions.js'; import { ContextKeyExpr } from '../../../../platform/contextkey/common/contextkey.js'; import { ITextEditorOptions, TextEditorSelectionRevealType } from '../../../../platform/editor/common/editor.js'; import { ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js'; @@ -26,6 +26,14 @@ import { IEditorService, SIDE_GROUP } from '../../../services/editor/common/edit import { ActiveEditorContext, IsSessionsWindowContext } from '../../../common/contextkeys.js'; import { createMultiDiffEditorLayoutDebugModel, isMultiDiffEditorLayoutDebugStateProvider } from './multiDiffEditorLayoutDebug.js'; +MenuRegistry.appendMenuItem(MenuId.EditorTitle, { + submenu: MenuId.DiffEditorViewSubmenu, + title: localize('diffView', "Diff View"), + group: '1_diff', + order: 10, + when: ContextKeyExpr.and(ActiveEditorContext.isEqualTo(MultiDiffEditor.ID), IsSessionsWindowContext.toNegated()), +}); + export class GoToFileAction extends Action2 { constructor() { super({ diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts index 7e03fe3b97d4..020f697d2125 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts @@ -31,6 +31,7 @@ import { MultiDiffEditorViewModel } from '../../../../editor/browser/widget/mult import { IMultiDiffEditorLayoutDebugState, IMultiDiffEditorViewState } from '../../../../editor/browser/widget/multiDiffEditor/multiDiffEditorWidgetImpl.js'; import { ICodeEditor } from '../../../../editor/browser/editorBrowser.js'; import { IDiffEditor } from '../../../../editor/common/editorCommon.js'; +import { DiffEditorViewMode } from '../../../../editor/common/config/editorOptions.js'; import { IMultiDiffEditorOptions } from '../../../../editor/common/multiDiffEditor.js'; import { Range } from '../../../../editor/common/core/range.js'; import { MultiDiffEditorItem } from './multiDiffSourceResolverService.js'; @@ -51,6 +52,10 @@ export class MultiDiffEditor extends AbstractEditorWithViewState { function createDiffEditorControl(model: ITextModel | undefined, originalFocused: boolean, modifiedFocused: boolean): IDiffEditor { return new class extends mock() { + override getEditorType() { return EditorType.IDiffEditor; } override getOriginalEditor() { return createOriginalEditor(originalFocused); } override getModifiedEditor() { return createModifiedEditor(model, modifiedFocused); } override goToDiff(target: 'next' | 'previous') { goToDiffCalls.push(target); } @@ -172,4 +174,30 @@ suite('DiffEditorCommandsService', () => { resetWidthBasedLayoutCalls: 1, }); }); + + test('sets the view mode for a multi-diff editor pane and resets all embedded layouts', async () => { + const model = { uri: URI.file('/foo.txt') } as ITextModel; + const control = createDiffEditorControl(model, false, false); + let resetAllCalls = 0; + const pane = new class extends mock() { + override getControl() { return control; } + resetDiffEditorWidthBasedLayout(): void { resetAllCalls++; } + }; + const { service, resourceWrites } = createService(pane); + + await service.setViewMode([], 'automatic'); + + assert.deepStrictEqual({ + resourceWrites, + resetAllCalls, + resetActiveControlCalls: resetWidthBasedLayoutCalls, + }, { + resourceWrites: [ + { resource: model.uri, key: 'diffEditor.renderSideBySide', value: true }, + { resource: model.uri, key: 'diffEditor.useInlineViewWhenSpaceIsLimited', value: true }, + ], + resetAllCalls: 1, + resetActiveControlCalls: 0, + }); + }); });