Skip to content

Commit

Permalink
Diff editor breaks when running vscode.diff twice in a row (fixes #…
Browse files Browse the repository at this point in the history 
…33593)
  • Loading branch information
@bpasero
bpasero committed Sep 4, 2017
1 parent ba54a99 commit 3d81fe6
Show file tree
Hide file tree
Showing 4 changed files with 196 additions and 204 deletions.
72 changes: 35 additions & 37 deletions src/vs/workbench/browser/parts/editor/binaryEditor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,48 +64,46 @@ export abstract class BaseBinaryResourceEditor extends BaseEditor {
}

public setInput(input: EditorInput, options?: EditorOptions): TPromise<void> {
const oldInput = this.input;
super.setInput(input, options);

// Detect options
// Return early for same input unless we force to open
const forceOpen = options && options.forceOpen;

// Same Input
if (!forceOpen && input.matches(oldInput)) {
if (!forceOpen && input.matches(this.input)) {
return TPromise.as<void>(null);
}

// Different Input (Reload)
return input.resolve(true).then((resolvedModel: EditorModel) => {

// Assert Model instance
if (!(resolvedModel instanceof BinaryEditorModel)) {
return TPromise.wrapError<void>(new Error('Unable to open file as binary'));
}

// Assert that the current input is still the one we expect. This prevents a race condition when loading takes long and another input was set meanwhile
if (!this.input || this.input !== input) {
return null;
}

// Render Input
const model = <BinaryEditorModel>resolvedModel;
ResourceViewer.show(
{ name: model.getName(), resource: model.getResource(), size: model.getSize(), etag: model.getETag() },
this.binaryContainer,
this.scrollbar,
(resource: URI) => {
this.windowsService.openExternal(resource.toString()).then(didOpen => {
if (!didOpen) {
return this.windowsService.showItemInFolder(resource.fsPath);
}

return void 0;
});
},
(meta) => this.handleMetadataChanged(meta));

return TPromise.as<void>(null);
// Otherwise set input and resolve
return super.setInput(input, options).then(() => {
return input.resolve(true).then((resolvedModel: EditorModel) => {

// Assert Model instance
if (!(resolvedModel instanceof BinaryEditorModel)) {
return TPromise.wrapError<void>(new Error('Unable to open file as binary'));
}

// Assert that the current input is still the one we expect. This prevents a race condition when loading takes long and another input was set meanwhile
if (!this.input || this.input !== input) {
return null;
}

// Render Input
const model = <BinaryEditorModel>resolvedModel;
ResourceViewer.show(
{ name: model.getName(), resource: model.getResource(), size: model.getSize(), etag: model.getETag() },
this.binaryContainer,
this.scrollbar,
(resource: URI) => {
this.windowsService.openExternal(resource.toString()).then(didOpen => {
if (!didOpen) {
return this.windowsService.showItemInFolder(resource.fsPath);
}

return void 0;
});
},
(meta) => this.handleMetadataChanged(meta));

return TPromise.as<void>(null);
});
});
}

Expand Down
84 changes: 41 additions & 43 deletions src/vs/workbench/browser/parts/editor/textDiffEditor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,16 +112,12 @@ export class TextDiffEditor extends BaseTextEditor {
}

public setInput(input: EditorInput, options?: EditorOptions): TPromise<void> {
const oldInput = this.input;
super.setInput(input, options);

// Detect options
// Return early for same input unless we force to open
const forceOpen = options && options.forceOpen;
if (!forceOpen && input.matches(this.input)) {

// Same Input
if (!forceOpen && input.matches(oldInput)) {

// TextOptions (avoiding instanceof here for a reason, do not change!)
// Still apply options if any (avoiding instanceof here for a reason, do not change!)
const textOptions = <TextEditorOptions>options;
if (textOptions && types.isFunction(textOptions.apply)) {
textOptions.apply(<IDiffEditor>this.getControl(), ScrollType.Smooth);
Expand All @@ -135,49 +131,51 @@ export class TextDiffEditor extends BaseTextEditor {
this.diffNavigator.dispose();
}

// Different Input (Reload)
return input.resolve(true).then(resolvedModel => {
// Set input and resolve
return super.setInput(input, options).then(() => {
return input.resolve(true).then(resolvedModel => {

// Assert Model Instance
if (!(resolvedModel instanceof TextDiffEditorModel) && this.openAsBinary(input, options)) {
return null;
}
// Assert Model Instance
if (!(resolvedModel instanceof TextDiffEditorModel) && this.openAsBinary(input, options)) {
return null;
}

// Assert that the current input is still the one we expect. This prevents a race condition when loading a diff takes long and another input was set meanwhile
if (!this.input || this.input !== input) {
return null;
}
// Assert that the current input is still the one we expect. This prevents a race condition when loading a diff takes long and another input was set meanwhile
if (!this.input || this.input !== input) {
return null;
}

// Editor
const diffEditor = <IDiffEditor>this.getControl();
diffEditor.setModel((<TextDiffEditorModel>resolvedModel).textDiffEditorModel);
// Editor
const diffEditor = <IDiffEditor>this.getControl();
diffEditor.setModel((<TextDiffEditorModel>resolvedModel).textDiffEditorModel);

// Handle TextOptions
let alwaysRevealFirst = true;
if (options && types.isFunction((<TextEditorOptions>options).apply)) {
const hadOptions = (<TextEditorOptions>options).apply(<IDiffEditor>diffEditor, ScrollType.Immediate);
if (hadOptions) {
alwaysRevealFirst = false; // Do not reveal if we are instructed to open specific line/col
// Handle TextOptions
let alwaysRevealFirst = true;
if (options && types.isFunction((<TextEditorOptions>options).apply)) {
const hadOptions = (<TextEditorOptions>options).apply(<IDiffEditor>diffEditor, ScrollType.Immediate);
if (hadOptions) {
alwaysRevealFirst = false; // Do not reveal if we are instructed to open specific line/col
}
}
}

// Listen on diff updated changes to reveal the first change
this.diffNavigator = new DiffNavigator(diffEditor, {
alwaysRevealFirst
});
this.diffNavigator.addListener(DiffNavigator.Events.UPDATED, () => {
this.nextDiffAction.updateEnablement();
this.previousDiffAction.updateEnablement();
});
}, error => {

// In case we tried to open a file and the response indicates that this is not a text file, fallback to binary diff.
if (this.isFileBinaryError(error) && this.openAsBinary(input, options)) {
return null;
}
// Listen on diff updated changes to reveal the first change
this.diffNavigator = new DiffNavigator(diffEditor, {
alwaysRevealFirst
});
this.diffNavigator.addListener(DiffNavigator.Events.UPDATED, () => {
this.nextDiffAction.updateEnablement();
this.previousDiffAction.updateEnablement();
});
}, error => {

// In case we tried to open a file and the response indicates that this is not a text file, fallback to binary diff.
if (this.isFileBinaryError(error) && this.openAsBinary(input, options)) {
return null;
}

// Otherwise make sure the error bubbles up
return TPromise.wrapError(error);
// Otherwise make sure the error bubbles up
return TPromise.wrapError(error);
});
});
}

Expand Down
76 changes: 37 additions & 39 deletions src/vs/workbench/browser/parts/editor/textResourceEditor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,12 @@ export class TextResourceEditor extends BaseTextEditor {
}

public setInput(input: EditorInput, options?: EditorOptions): TPromise<void> {
const oldInput = this.input;
super.setInput(input, options);

// Detect options
// Return early for same input unless we force to open
const forceOpen = options && options.forceOpen;
if (!forceOpen && input.matches(this.input)) {

// Same Input
if (!forceOpen && input.matches(oldInput)) {

// TextOptions (avoiding instanceof here for a reason, do not change!)
// Still apply options if any (avoiding instanceof here for a reason, do not change!)
const textOptions = <TextEditorOptions>options;
if (textOptions && types.isFunction(textOptions.apply)) {
textOptions.apply(this.getControl(), ScrollType.Smooth);
Expand All @@ -74,39 +70,41 @@ export class TextResourceEditor extends BaseTextEditor {
}

// Remember view settings if input changes
this.saveTextEditorViewStateForInput(oldInput);

// Different Input (Reload)
return input.resolve(true).then((resolvedModel: EditorModel) => {

// Assert Model instance
if (!(resolvedModel instanceof BaseTextEditorModel)) {
return TPromise.wrapError<void>(new Error('Unable to open file as text'));
}

// Assert that the current input is still the one we expect. This prevents a race condition when loading takes long and another input was set meanwhile
if (!this.input || this.input !== input) {
return null;
}

// Set Editor Model
const textEditor = this.getControl();
const textEditorModel = resolvedModel.textEditorModel;
textEditor.setModel(textEditorModel);

// Apply Options from TextOptions
let optionsGotApplied = false;
const textOptions = <TextEditorOptions>options;
if (textOptions && types.isFunction(textOptions.apply)) {
optionsGotApplied = textOptions.apply(textEditor, ScrollType.Immediate);
}

// Otherwise restore View State
if (!optionsGotApplied) {
this.restoreViewState(input);
}
this.saveTextEditorViewStateForInput(this.input);

return void 0;
// Set input and resolve
return super.setInput(input, options).then(() => {
return input.resolve(true).then((resolvedModel: EditorModel) => {

// Assert Model instance
if (!(resolvedModel instanceof BaseTextEditorModel)) {
return TPromise.wrapError<void>(new Error('Unable to open file as text'));
}

// Assert that the current input is still the one we expect. This prevents a race condition when loading takes long and another input was set meanwhile
if (!this.input || this.input !== input) {
return null;
}

// Set Editor Model
const textEditor = this.getControl();
const textEditorModel = resolvedModel.textEditorModel;
textEditor.setModel(textEditorModel);

// Apply Options from TextOptions
let optionsGotApplied = false;
const textOptions = <TextEditorOptions>options;
if (textOptions && types.isFunction(textOptions.apply)) {
optionsGotApplied = textOptions.apply(textEditor, ScrollType.Immediate);
}

// Otherwise restore View State
if (!optionsGotApplied) {
this.restoreViewState(input);
}

return void 0;
});
});
}

Expand Down

0 comments on commit 3d81fe6

Please sign in to comment.