Skip to content

Commit

Permalink
introduce --merge to bring up merge editor (for #5770) (#155039)
Browse files Browse the repository at this point in the history
* introduce `--merge` to bring up merge editor (for #5770)

* wait on proper editor when merging

* sqlite slowness

* disable flush on write in tests unless disk tests

* more runWithFakedTimers

* disable flush also in pfs

* introduce `IResourceMergeEditorInput`

* cleanup

* align with merge editor names

* stronger check

* adopt `ResourceSet`

* no need to coalesce

* improve `matches` method
  • Loading branch information
bpasero committed Jul 18, 2022
1 parent 03c16c9 commit a567b59
Show file tree
Hide file tree
Showing 33 changed files with 494 additions and 197 deletions.
1 change: 1 addition & 0 deletions src/vs/code/electron-main/app.ts
Expand Up @@ -1012,6 +1012,7 @@ export class CodeApplication extends Disposable {
cli: args,
forceNewWindow: args['new-window'] || (!hasCliArgs && args['unity-launch']),
diffMode: args.diff,
mergeMode: args.merge,
noRecentEntry,
waitMarkerFileURI,
gotoLineMode: args.goto,
Expand Down
1 change: 1 addition & 0 deletions src/vs/platform/environment/common/argv.ts
Expand Up @@ -18,6 +18,7 @@ export interface NativeParsedArgs {
wait?: boolean;
waitMarkerFilePath?: string;
diff?: boolean;
merge?: boolean;
add?: boolean;
goto?: boolean;
'new-window'?: boolean;
Expand Down
1 change: 1 addition & 0 deletions src/vs/platform/environment/node/argv.ts
Expand Up @@ -41,6 +41,7 @@ type OptionTypeName<T> =

export const OPTIONS: OptionDescriptions<Required<NativeParsedArgs>> = {
'diff': { type: 'boolean', cat: 'o', alias: 'd', args: ['file', 'file'], description: localize('diff', "Compare two files with each other.") },
'merge': { type: 'boolean', cat: 'o', alias: 'm', args: ['path1', 'path2', 'base', 'result'], description: localize('merge', "Perform a three-way merge by providing paths for two modified versions of a file, the common origin of both modified versions and the output file to save merge results.") },
'add': { type: 'boolean', cat: 'o', alias: 'a', args: 'folder', description: localize('add', "Add folder(s) to the last active window.") },
'goto': { type: 'boolean', cat: 'o', alias: 'g', args: 'file:line[:character]', description: localize('goto', "Open a file at the path on the specified line and character position.") },
'new-window': { type: 'boolean', cat: 'o', alias: 'n', description: localize('newWindow', "Force to open a new window.") },
Expand Down
1 change: 1 addition & 0 deletions src/vs/platform/launch/electron-main/launchMainService.ts
Expand Up @@ -190,6 +190,7 @@ export class LaunchMainService implements ILaunchMainService {
preferNewWindow: !args['reuse-window'] && !args.wait,
forceReuseWindow: args['reuse-window'],
diffMode: args.diff,
mergeMode: args.merge,
addMode: args.add,
noRecentEntry: !!args['skip-add-to-recently-opened'],
gotoLineMode: args.goto
Expand Down
Expand Up @@ -154,6 +154,7 @@ export class NativeHostMainService extends Disposable implements INativeHostMain
forceReuseWindow: options.forceReuseWindow,
preferNewWindow: options.preferNewWindow,
diffMode: options.diffMode,
mergeMode: options.mergeMode,
addMode: options.addMode,
gotoLineMode: options.gotoLineMode,
noRecentEntry: options.noRecentEntry,
Expand Down
3 changes: 3 additions & 0 deletions src/vs/platform/window/common/window.ts
Expand Up @@ -52,6 +52,7 @@ export interface IOpenWindowOptions extends IBaseOpenWindowsOptions {
readonly addMode?: boolean;

readonly diffMode?: boolean;
readonly mergeMode?: boolean;
readonly gotoLineMode?: boolean;

readonly waitMarkerFileURI?: URI;
Expand Down Expand Up @@ -240,6 +241,7 @@ interface IPathsToWaitForData {
export interface IOpenFileRequest {
readonly filesToOpenOrCreate?: IPathData[];
readonly filesToDiff?: IPathData[];
readonly filesToMerge?: IPathData[];
}

/**
Expand Down Expand Up @@ -270,6 +272,7 @@ export interface IWindowConfiguration {

filesToOpenOrCreate?: IPath[];
filesToDiff?: IPath[];
filesToMerge?: IPath[];
}

export interface IOSConfiguration {
Expand Down
1 change: 1 addition & 0 deletions src/vs/platform/windows/electron-main/window.ts
Expand Up @@ -899,6 +899,7 @@ export class CodeWindow extends Disposable implements ICodeWindow {
// Delete some properties we do not want during reload
delete configuration.filesToOpenOrCreate;
delete configuration.filesToDiff;
delete configuration.filesToMerge;
delete configuration.filesToWait;

// Some configuration things get inherited if the window is being reloaded and we are
Expand Down
1 change: 1 addition & 0 deletions src/vs/platform/windows/electron-main/windows.ts
Expand Up @@ -85,6 +85,7 @@ export interface IOpenConfiguration extends IBaseOpenConfiguration {
readonly forceReuseWindow?: boolean;
readonly forceEmpty?: boolean;
readonly diffMode?: boolean;
readonly mergeMode?: boolean;
addMode?: boolean;
readonly gotoLineMode?: boolean;
readonly initialStartup?: boolean;
Expand Down
31 changes: 24 additions & 7 deletions src/vs/platform/windows/electron-main/windowsMainService.ts
Expand Up @@ -118,6 +118,8 @@ interface IFilesToOpen {

filesToOpenOrCreate: IPath[];
filesToDiff: IPath[];
filesToMerge: IPath[];

filesToWait?: IPathsToWaitFor;
}

Expand Down Expand Up @@ -296,7 +298,7 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic
workspacesToOpen.push(path);
} else if (path.fileUri) {
if (!filesToOpen) {
filesToOpen = { filesToOpenOrCreate: [], filesToDiff: [], remoteAuthority: path.remoteAuthority };
filesToOpen = { filesToOpenOrCreate: [], filesToDiff: [], filesToMerge: [], remoteAuthority: path.remoteAuthority };
}
filesToOpen.filesToOpenOrCreate.push(path);
} else if (path.backupPath) {
Expand All @@ -312,9 +314,16 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic
filesToOpen.filesToOpenOrCreate = [];
}

// When run with --merge, take the first 4 files to open as files to merge
if (openConfig.mergeMode && filesToOpen && filesToOpen.filesToOpenOrCreate.length === 4) {
filesToOpen.filesToMerge = filesToOpen.filesToOpenOrCreate.slice(0, 4);
filesToOpen.filesToOpenOrCreate = [];
filesToOpen.filesToDiff = [];
}

// When run with --wait, make sure we keep the paths to wait for
if (filesToOpen && openConfig.waitMarkerFileURI) {
filesToOpen.filesToWait = { paths: [...filesToOpen.filesToDiff, ...filesToOpen.filesToOpenOrCreate], waitMarkerFileUri: openConfig.waitMarkerFileURI };
filesToOpen.filesToWait = { paths: [...filesToOpen.filesToDiff, filesToOpen.filesToMerge[3] /* [3] is the resulting merge file */, ...filesToOpen.filesToOpenOrCreate], waitMarkerFileUri: openConfig.waitMarkerFileURI };
}

// These are windows to restore because of hot-exit or from previous session (only performed once on startup!)
Expand Down Expand Up @@ -384,9 +393,10 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic
}

// Remember in recent document list (unless this opens for extension development)
// Also do not add paths when files are opened for diffing, only if opened individually
// Also do not add paths when files are opened for diffing or merging, only if opened individually
const isDiff = filesToOpen && filesToOpen.filesToDiff.length > 0;
if (!usedWindows.some(window => window.isExtensionDevelopmentHost) && !isDiff && !openConfig.noRecentEntry) {
const isMerge = filesToOpen && filesToOpen.filesToMerge.length > 0;
if (!usedWindows.some(window => window.isExtensionDevelopmentHost) && !isDiff && !isMerge && !openConfig.noRecentEntry) {
const recents: IRecent[] = [];
for (const pathToOpen of pathsToOpen) {
if (isWorkspacePathToOpen(pathToOpen) && !pathToOpen.transient /* never add transient workspaces to history */) {
Expand Down Expand Up @@ -461,13 +471,13 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic
}
}

// Handle files to open/diff or to create when we dont open a folder and we do not restore any
// Handle files to open/diff/merge or to create when we dont open a folder and we do not restore any
// folder/untitled from hot-exit by trying to open them in the window that fits best
const potentialNewWindowsCount = foldersToOpen.length + workspacesToOpen.length + emptyToRestore.length;
if (filesToOpen && potentialNewWindowsCount === 0) {

// Find suitable window or folder path to open files in
const fileToCheck = filesToOpen.filesToOpenOrCreate[0] || filesToOpen.filesToDiff[0];
const fileToCheck = filesToOpen.filesToOpenOrCreate[0] || filesToOpen.filesToDiff[0] || filesToOpen.filesToMerge[3] /* [3] is the resulting merge file */;

// only look at the windows with correct authority
const windows = this.getWindows().filter(window => filesToOpen && isEqualAuthority(window.remoteAuthority, filesToOpen.remoteAuthority));
Expand Down Expand Up @@ -625,6 +635,7 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic
const params: INativeOpenFileRequest = {
filesToOpenOrCreate: filesToOpen?.filesToOpenOrCreate,
filesToDiff: filesToOpen?.filesToDiff,
filesToMerge: filesToOpen?.filesToMerge,
filesToWait: filesToOpen?.filesToWait,
termProgram: configuration?.userEnv?.['TERM_PROGRAM']
};
Expand Down Expand Up @@ -792,7 +803,12 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic
ignoreFileNotFound: true,
gotoLineMode: cli.goto,
remoteAuthority: cli.remote || undefined,
forceOpenWorkspaceAsFile: cli.diff && cli._.length === 2 // special case diff mode to force open workspace as file (https://github.com/microsoft/vscode/issues/149731)
forceOpenWorkspaceAsFile:
// special case diff / merge mode to force open
// workspace as file
// https://github.com/microsoft/vscode/issues/149731
cli.diff && cli._.length === 2 ||
cli.merge && cli._.length === 4
};

// folder uris
Expand Down Expand Up @@ -1323,6 +1339,7 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic

filesToOpenOrCreate: options.filesToOpen?.filesToOpenOrCreate,
filesToDiff: options.filesToOpen?.filesToDiff,
filesToMerge: options.filesToOpen?.filesToMerge,
filesToWait: options.filesToOpen?.filesToWait,

logLevel: this.logService.getLevel(),
Expand Down
2 changes: 2 additions & 0 deletions src/vs/server/node/server.cli.ts
Expand Up @@ -59,6 +59,7 @@ const isSupportedForPipe = (optionId: keyof RemoteParsedArgs) => {
case 'file-uri':
case 'add':
case 'diff':
case 'merge':
case 'wait':
case 'goto':
case 'reuse-window':
Expand Down Expand Up @@ -297,6 +298,7 @@ export function main(desc: ProductDescription, args: string[]): void {
fileURIs,
folderURIs,
diffMode: parsedArgs.diff,
mergeMode: parsedArgs.merge,
addMode: parsedArgs.add,
gotoLineMode: parsedArgs.goto,
forceReuseWindow: parsedArgs['reuse-window'],
Expand Down
5 changes: 3 additions & 2 deletions src/vs/workbench/api/node/extHostCLIServer.ts
Expand Up @@ -18,6 +18,7 @@ export interface OpenCommandPipeArgs {
folderURIs?: string[];
forceNewWindow?: boolean;
diffMode?: boolean;
mergeMode?: boolean;
addMode?: boolean;
gotoLineMode?: boolean;
forceReuseWindow?: boolean;
Expand Down Expand Up @@ -118,7 +119,7 @@ export class CLIServerBase {
}

private async open(data: OpenCommandPipeArgs): Promise<string> {
const { fileURIs, folderURIs, forceNewWindow, diffMode, addMode, forceReuseWindow, gotoLineMode, waitMarkerFilePath, remoteAuthority } = data;
const { fileURIs, folderURIs, forceNewWindow, diffMode, mergeMode, addMode, forceReuseWindow, gotoLineMode, waitMarkerFilePath, remoteAuthority } = data;
const urisToOpen: IWindowOpenable[] = [];
if (Array.isArray(folderURIs)) {
for (const s of folderURIs) {
Expand All @@ -144,7 +145,7 @@ export class CLIServerBase {
}
const waitMarkerFileURI = waitMarkerFilePath ? URI.file(waitMarkerFilePath) : undefined;
const preferNewWindow = !forceReuseWindow && !waitMarkerFileURI && !addMode;
const windowOpenArgs: IOpenWindowOptions = { forceNewWindow, diffMode, addMode, gotoLineMode, forceReuseWindow, preferNewWindow, waitMarkerFileURI, remoteAuthority };
const windowOpenArgs: IOpenWindowOptions = { forceNewWindow, diffMode, mergeMode, addMode, gotoLineMode, forceReuseWindow, preferNewWindow, waitMarkerFileURI, remoteAuthority };
this._commands.executeCommand('_remoteCLI.windowOpen', urisToOpen, windowOpenArgs);

return '';
Expand Down
72 changes: 37 additions & 35 deletions src/vs/workbench/browser/layout.ts
Expand Up @@ -9,7 +9,7 @@ import { EventType, addDisposableListener, getClientArea, Dimension, position, s
import { onDidChangeFullscreen, isFullscreen } from 'vs/base/browser/browser';
import { IWorkingCopyBackupService } from 'vs/workbench/services/workingCopy/common/workingCopyBackup';
import { isWindows, isLinux, isMacintosh, isWeb, isNative, isIOS } from 'vs/base/common/platform';
import { IUntypedEditorInput, pathsToEditors } from 'vs/workbench/common/editor';
import { isResourceEditorInput, IUntypedEditorInput, pathsToEditors } from 'vs/workbench/common/editor';
import { SideBySideEditorInput } from 'vs/workbench/common/editor/sideBySideEditorInput';
import { SidebarPart } from 'vs/workbench/browser/parts/sidebar/sidebarPart';
import { PanelPart } from 'vs/workbench/browser/parts/panel/panelPart';
Expand Down Expand Up @@ -75,7 +75,7 @@ interface IWorkbenchLayoutWindowInitializationState {
};
editor: {
restoreEditors: boolean;
editorsToOpen: Promise<IUntypedEditorInput[]> | IUntypedEditorInput[];
editorsToOpen: Promise<IUntypedEditorInput[]>;
};
}

Expand All @@ -98,6 +98,7 @@ enum WorkbenchLayoutClasses {
interface IInitialFilesToOpen {
filesToOpenOrCreate?: IPath[];
filesToDiff?: IPath[];
filesToMerge?: IPath[];
}

export abstract class Layout extends Disposable implements IWorkbenchLayoutService {
Expand Down Expand Up @@ -278,7 +279,7 @@ export abstract class Layout extends Disposable implements IWorkbenchLayoutServi
this._register(this.hostService.onDidChangeFocus(e => this.onWindowFocusChanged(e)));
}

private onMenubarToggled(visible: boolean) {
private onMenubarToggled(visible: boolean): void {
if (visible !== this.windowState.runtime.menuBar.toggled) {
this.windowState.runtime.menuBar.toggled = visible;

Expand Down Expand Up @@ -565,26 +566,33 @@ export abstract class Layout extends Disposable implements IWorkbenchLayoutServi
return this.windowState.initialization.editor.restoreEditors;
}

private resolveEditorsToOpen(fileService: IFileService, initialFilesToOpen: IInitialFilesToOpen | undefined): Promise<IUntypedEditorInput[]> | IUntypedEditorInput[] {

// Files to open, diff or create
private async resolveEditorsToOpen(fileService: IFileService, initialFilesToOpen: IInitialFilesToOpen | undefined): Promise<IUntypedEditorInput[]> {
if (initialFilesToOpen) {

// Files to diff is exclusive
return pathsToEditors(initialFilesToOpen.filesToDiff, fileService).then(filesToDiff => {
if (filesToDiff.length === 2) {
const diffEditorInput: IUntypedEditorInput[] = [{
original: { resource: filesToDiff[0].resource },
modified: { resource: filesToDiff[1].resource },
options: { pinned: true }
}];
// Merge editor
const filesToMerge = await pathsToEditors(initialFilesToOpen.filesToMerge, fileService);
if (filesToMerge.length === 4 && isResourceEditorInput(filesToMerge[0]) && isResourceEditorInput(filesToMerge[1]) && isResourceEditorInput(filesToMerge[2]) && isResourceEditorInput(filesToMerge[3])) {
return [{
input1: { resource: filesToMerge[0].resource },
input2: { resource: filesToMerge[1].resource },
base: { resource: filesToMerge[2].resource },
result: { resource: filesToMerge[3].resource },
options: { pinned: true, override: 'mergeEditor.Input' } // TODO@bpasero remove the override once the resolver is ready
}];
}

return diffEditorInput;
}
// Diff editor
const filesToDiff = await pathsToEditors(initialFilesToOpen.filesToDiff, fileService);
if (filesToDiff.length === 2) {
return [{
original: { resource: filesToDiff[0].resource },
modified: { resource: filesToDiff[1].resource },
options: { pinned: true }
}];
}

// Otherwise: Open/Create files
return pathsToEditors(initialFilesToOpen.filesToOpenOrCreate, fileService);
});
// Normal editor
return pathsToEditors(initialFilesToOpen.filesToOpenOrCreate, fileService);
}

// Empty workbench configured to open untitled file if empty
Expand All @@ -593,13 +601,12 @@ export abstract class Layout extends Disposable implements IWorkbenchLayoutServi
return []; // do not open any empty untitled file if we restored groups/editors from previous session
}

return this.workingCopyBackupService.hasBackups().then(hasBackups => {
if (hasBackups) {
return []; // do not open any empty untitled file if we have backups to restore
}
const hasBackups = await this.workingCopyBackupService.hasBackups();
if (hasBackups) {
return []; // do not open any empty untitled file if we have backups to restore
}

return [{ resource: undefined }]; // open empty untitled file
});
return [{ resource: undefined }]; // open empty untitled file
}

return [];
Expand Down Expand Up @@ -638,10 +645,10 @@ export abstract class Layout extends Disposable implements IWorkbenchLayoutServi
};
}

// Then check for files to open, create or diff from main side
const { filesToOpenOrCreate, filesToDiff } = this.environmentService;
if (filesToOpenOrCreate || filesToDiff) {
return { filesToOpenOrCreate, filesToDiff };
// Then check for files to open, create or diff/merge from main side
const { filesToOpenOrCreate, filesToDiff, filesToMerge } = this.environmentService;
if (filesToOpenOrCreate || filesToDiff || filesToMerge) {
return { filesToOpenOrCreate, filesToDiff, filesToMerge };
}

return undefined;
Expand Down Expand Up @@ -681,12 +688,7 @@ export abstract class Layout extends Disposable implements IWorkbenchLayoutServi
// signaling that layout is restored, but we do
// not need to await the editors from having
// fully loaded.
let editors: IUntypedEditorInput[];
if (Array.isArray(this.windowState.initialization.editor.editorsToOpen)) {
editors = this.windowState.initialization.editor.editorsToOpen;
} else {
editors = await this.windowState.initialization.editor.editorsToOpen;
}
const editors = await this.windowState.initialization.editor.editorsToOpen;

let openEditorsPromise: Promise<unknown> | undefined = undefined;
if (editors.length) {
Expand Down

2 comments on commit a567b59

@gjsjohnmurray
Copy link
Contributor

Choose a reason for hiding this comment

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

@bpasero Apologies for subverting this commit, but I wonder if it's time the locked issue #37350 got closed.

@bpasero
Copy link
Member Author

Choose a reason for hiding this comment

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

I think that is up to merge editor owners and so far the merge editor is not enabled by default yet (as far as I know).

However I will be closing another popular issue as a result soon: #5770

Please sign in to comment.