Skip to content

Commit

Permalink
Fix for console warnings on merge microsoft#27584.
Browse files Browse the repository at this point in the history
merge-conflict edit commands are falsey async. They generated an edit from the editor to work around the supplied TextEditorEdit being ignored if execution was not synchronous. This change creates a sync code path for getting document conflicts, the reality the await aync get document conflicts would unlikely ever be "delayed" and shared between multiple invocations on the command execution path. It'll happen "some time" after initial scanning completes.
  • Loading branch information
pprice committed May 31, 2017
1 parent b5a1ecb commit 86733f6
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 33 deletions.
63 changes: 36 additions & 27 deletions extensions/merge-conflict/src/commandHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,36 +43,36 @@ export default class CommandHandler implements vscode.Disposable {
);
}

acceptCurrent(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, ...args): Promise<void> {
return this.accept(interfaces.CommitType.Current, editor, ...args);
acceptCurrent(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, ...args) {
return this.accept(interfaces.CommitType.Current, editor, edit, ...args);
}

acceptIncoming(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, ...args): Promise<void> {
return this.accept(interfaces.CommitType.Incoming, editor, ...args);
acceptIncoming(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, ...args) {
return this.accept(interfaces.CommitType.Incoming, editor, edit, ...args);
}

acceptBoth(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, ...args): Promise<void> {
return this.accept(interfaces.CommitType.Both, editor, ...args);
acceptBoth(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, ...args) {
return this.accept(interfaces.CommitType.Both, editor, edit, ...args);
}

acceptAllCurrent(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, ...args): Promise<void> {
return this.acceptAll(interfaces.CommitType.Current, editor);
acceptAllCurrent(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, ...args) {
return this.acceptAll(interfaces.CommitType.Current, editor, edit);
}

acceptAllIncoming(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, ...args): Promise<void> {
return this.acceptAll(interfaces.CommitType.Incoming, editor);
acceptAllIncoming(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, ...args) {
return this.acceptAll(interfaces.CommitType.Incoming, editor, edit);
}

acceptAllBoth(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, ...args): Promise<void> {
return this.acceptAll(interfaces.CommitType.Both, editor);
acceptAllBoth(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, ...args) {
return this.acceptAll(interfaces.CommitType.Both, editor, edit);
}

async compare(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, conflict: interfaces.IDocumentMergeConflict | null, ...args) {
compare(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, conflict: interfaces.IDocumentMergeConflict | null, ...args) {
const fileName = path.basename(editor.document.uri.fsPath);

// No conflict, command executed from command palette
if (!conflict) {
conflict = await this.findConflictContainingSelection(editor);
conflict = this.findConflictContainingSelection(editor);

// Still failed to find conflict, warn the user and exit
if (!conflict) {
Expand Down Expand Up @@ -102,8 +102,8 @@ export default class CommandHandler implements vscode.Disposable {
return this.navigate(editor, NavigationDirection.Backwards);
}

async acceptSelection(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, ...args): Promise<void> {
let conflict = await this.findConflictContainingSelection(editor);
acceptSelection(editor: vscode.TextEditor, edit: vscode.TextEditorEdit, ...args) {
let conflict = this.findConflictContainingSelection(editor);

if (!conflict) {
vscode.window.showWarningMessage(localize('cursorNotInConflict', 'Editor cursor is not within a merge conflict'));
Expand All @@ -129,7 +129,7 @@ export default class CommandHandler implements vscode.Disposable {
}

this.tracker.forget(editor.document);
conflict.commitEdit(typeToAccept, editor);
conflict.commitEdit(typeToAccept, editor, edit);
}

dispose() {
Expand Down Expand Up @@ -158,7 +158,7 @@ export default class CommandHandler implements vscode.Disposable {
editor.revealRange(navigationResult.conflict.range, vscode.TextEditorRevealType.Default);
}

private async accept(type: interfaces.CommitType, editor: vscode.TextEditor, ...args): Promise<void> {
private accept(type: interfaces.CommitType, editor: vscode.TextEditor, edit: vscode.TextEditorEdit | undefined, ...args) {

let conflict: interfaces.IDocumentMergeConflict | null;

Expand All @@ -168,7 +168,7 @@ export default class CommandHandler implements vscode.Disposable {
}
else {
// Attempt to find a conflict that matches the current curosr position
conflict = await this.findConflictContainingSelection(editor);
conflict = this.findConflictContainingSelection(editor);
}

if (!conflict) {
Expand All @@ -178,11 +178,11 @@ export default class CommandHandler implements vscode.Disposable {

// Tracker can forget as we know we are going to do an edit
this.tracker.forget(editor.document);
conflict.commitEdit(type, editor);
conflict.commitEdit(type, editor, edit);
}

private async acceptAll(type: interfaces.CommitType, editor: vscode.TextEditor): Promise<void> {
let conflicts = await this.tracker.getConflicts(editor.document);
private acceptAll(type: interfaces.CommitType, editor: vscode.TextEditor, edit: vscode.TextEditorEdit | undefined) {
let conflicts = this.tracker.getConflictsSync(editor.document);

if (!conflicts || conflicts.length === 0) {
vscode.window.showWarningMessage(localize('noConflicts', 'No merge conflicts found in this file'));
Expand All @@ -192,16 +192,25 @@ export default class CommandHandler implements vscode.Disposable {
// For get the current state of the document, as we know we are doing to do a large edit
this.tracker.forget(editor.document);

// Apply all changes as one edit
await editor.edit((edit) => conflicts.forEach(conflict => {
// Generate a "callback" that applies the specified action to all conflicts with the document
const allConflictsEdit = (edit) => conflicts.forEach(conflict => {
conflict.applyEdit(type, editor, edit);
}));
});

// We got an edit from the command system, use that.
if (edit) {
allConflictsEdit(edit);
return;
}

// No edit supplied, generate one
editor.edit(allConflictsEdit);
}

private async findConflictContainingSelection(editor: vscode.TextEditor, conflicts?: interfaces.IDocumentMergeConflict[]): Promise<interfaces.IDocumentMergeConflict | null> {
private findConflictContainingSelection(editor: vscode.TextEditor, conflicts?: interfaces.IDocumentMergeConflict[]): interfaces.IDocumentMergeConflict | null {

if (!conflicts) {
conflicts = await this.tracker.getConflicts(editor.document);
conflicts = this.tracker.getConflictsSync(editor.document);
}

if (!conflicts || conflicts.length === 0) {
Expand Down
8 changes: 3 additions & 5 deletions extensions/merge-conflict/src/documentMergeConflict.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,17 @@ export class DocumentMergeConflict implements interfaces.IDocumentMergeConflict
this.splitter = descriptor.splitter;
}

public commitEdit(type: interfaces.CommitType, editor: vscode.TextEditor, edit?: vscode.TextEditorEdit): Thenable<boolean> {
public async commitEdit(type: interfaces.CommitType, editor: vscode.TextEditor, edit?: vscode.TextEditorEdit): Promise<boolean> {

if (edit) {

this.applyEdit(type, editor, edit);
return Promise.resolve(true);
return true;
};

return editor.edit((edit) => this.applyEdit(type, editor, edit));
return await editor.edit((edit) => this.applyEdit(type, editor, edit));
}

public applyEdit(type: interfaces.CommitType, editor: vscode.TextEditor, edit: vscode.TextEditorEdit): void {

// Each conflict is a set of ranges as follows, note placements or newlines
// which may not in in spans
// [ Conflict Range -- (Entire content below)
Expand Down
8 changes: 8 additions & 0 deletions extensions/merge-conflict/src/documentTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ class OriginDocumentMergeConflictTracker implements interfaces.IDocumentMergeCon
return this.parent.getConflicts(document, this.origin);
}

getConflictsSync(document: vscode.TextDocument): interfaces.IDocumentMergeConflict[] {
return this.parent.getConflictsSync(document, this.origin);
}

isPending(document: vscode.TextDocument): boolean {
return this.parent.isPending(document, this.origin);
}
Expand All @@ -51,6 +55,10 @@ export default class DocumentMergeConflictTracker implements vscode.Disposable,
private cache: Map<string, ScanTask> = new Map();
private delayExpireTime: number = 250;

getConflictsSync(document: vscode.TextDocument, origin: string): interfaces.IDocumentMergeConflict[] {
return this.getConflictsOrEmpty(document, [origin]);
}

getConflicts(document: vscode.TextDocument, origin: string): PromiseLike<interfaces.IDocumentMergeConflict[]> {
// Attempt from cache

Expand Down
3 changes: 2 additions & 1 deletion extensions/merge-conflict/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export interface IExtensionConfiguration {
}

export interface IDocumentMergeConflict extends IDocumentMergeConflictDescriptor {
commitEdit(type: CommitType, editor: vscode.TextEditor, edit?: vscode.TextEditorEdit);
commitEdit(type: CommitType, editor: vscode.TextEditor, edit?: vscode.TextEditorEdit): Thenable<boolean>;
applyEdit(type: CommitType, editor: vscode.TextEditor, edit: vscode.TextEditorEdit);
}

Expand All @@ -37,6 +37,7 @@ export interface IDocumentMergeConflictDescriptor {

export interface IDocumentMergeConflictTracker {
getConflicts(document: vscode.TextDocument): PromiseLike<IDocumentMergeConflict[]>;
getConflictsSync(document: vscode.TextDocument): IDocumentMergeConflict[];
isPending(document: vscode.TextDocument): boolean;
forget(document: vscode.TextDocument);
}
Expand Down

0 comments on commit 86733f6

Please sign in to comment.