diff --git a/src/vs/workbench/parts/files/common/editors/textFileEditorModel.ts b/src/vs/workbench/parts/files/common/editors/textFileEditorModel.ts index d57c484c600..dd27a823a56 100644 --- a/src/vs/workbench/parts/files/common/editors/textFileEditorModel.ts +++ b/src/vs/workbench/parts/files/common/editors/textFileEditorModel.ts @@ -10,6 +10,7 @@ import {TPromise} from 'vs/base/common/winjs.base'; import {onUnexpectedError} from 'vs/base/common/errors'; import {toErrorMessage} from 'vs/base/common/errorMessage'; import URI from 'vs/base/common/uri'; +import * as assert from 'vs/base/common/assert'; import {IDisposable, dispose} from 'vs/base/common/lifecycle'; import paths = require('vs/base/common/paths'); import diagnostics = require('vs/base/common/diagnostics'); @@ -69,13 +70,10 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil ) { super(modelService, modeService); - this.toDispose = []; + assert.ok(resource.scheme === 'file', 'TextFileEditorModel can only handle file:// resources.'); this.resource = resource; - if (this.resource.scheme !== 'file') { - throw new Error('TextFileEditorModel can only handle file:// resources.'); - } - + this.toDispose = []; this._onDidStateChange = new Emitter(); this.toDispose.push(this._onDidStateChange); this.preferredEncoding = preferredEncoding; @@ -86,6 +84,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil this.mapPendingSaveToVersionId = {}; this.updateAutoSaveConfiguration(textFileService.getAutoSaveConfiguration()); + this.registerListeners(); } @@ -290,11 +289,11 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil } private onModelContentChanged(e: IModelContentChangedEvent): void { - diag('onModelContentChanged(' + e.changeType + ') - enter', this.resource, new Date()); + diag(`onModelContentChanged(${e.changeType}) - enter`, this.resource, new Date()); // In any case increment the version id because it tracks the textual content state of the model at all times this.versionId++; - diag('onModelContentChanged() - new versionId ' + this.versionId, this.resource, new Date()); + diag(`onModelContentChanged() - new versionId ${this.versionId}`, this.resource, new Date()); // Ignore if blocking model changes if (this.blockModelContentChange) { @@ -323,7 +322,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil diag('onModelContentChanged() - model content changed and marked as dirty', this.resource, new Date()); // Mark as dirty - this.makeDirty(e); + this.makeDirty(); // Start auto save process unless we are in conflict resolution mode and unless it is disabled if (this.autoSaveAfterMilliesEnabled) { @@ -335,7 +334,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil } } - private makeDirty(e?: IModelContentChangedEvent): void { + private makeDirty(): void { // Track dirty state and version id const wasDirty = this.dirty; @@ -348,13 +347,13 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil } private doAutoSave(versionId: number): TPromise { - diag('doAutoSave() - enter for versionId ' + versionId, this.resource, new Date()); + diag(`doAutoSave() - enter for versionId ${versionId}`, this.resource, new Date()); // Cancel any currently running auto saves to make this the one that succeeds this.cancelAutoSavePromises(); // Create new save promise and keep it - const promise: TPromise = TPromise.timeout(this.autoSaveAfterMillies).then(() => { + const promise = TPromise.timeout(this.autoSaveAfterMillies).then(() => { // Only trigger save if the version id has not changed meanwhile if (versionId === this.versionId) { @@ -390,27 +389,42 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil } private doSave(versionId: number, isAutoSaved: boolean, overwriteReadonly?: boolean, overwriteEncoding?: boolean): TPromise { - diag('doSave(' + versionId + ') - enter with versionId ' + versionId, this.resource, new Date()); + diag(`doSave(${versionId}) - enter with versionId ' + versionId`, this.resource, new Date()); // Lookup any running pending save for this versionId and return it if found + // + // Scenario: user invoked the save action multiple times quickly for the same contents + // while the save was not yet finished to disk + // const pendingSave = this.mapPendingSaveToVersionId[versionId]; if (pendingSave) { - diag('doSave(' + versionId + ') - exit - found a pending save for versionId ' + versionId, this.resource, new Date()); + diag(`doSave(${versionId}) - exit - found a pending save for versionId ${versionId}`, this.resource, new Date()); return pendingSave; } // Return early if not dirty or version changed meanwhile + // + // Scenario A: user invoked save action even though the model is not dirty + // Scenario B: auto save was triggered for a certain change by the user but meanwhile the user changed + // the contents and the version for which auto save was started is no longer the latest. + // Thus we avoid spawning multiple auto saves and only take the latest. + // if (!this.dirty || versionId !== this.versionId) { - diag('doSave(' + versionId + ') - exit - because not dirty and/or versionId is different (this.isDirty: ' + this.dirty + ', this.versionId: ' + this.versionId + ')', this.resource, new Date()); + diag(`doSave(${versionId}) - exit - because not dirty and/or versionId is different (this.isDirty: ${this.dirty}, this.versionId: ${this.versionId})`, this.resource, new Date()); return TPromise.as(null); } // Return if currently saving by scheduling another auto save. Never ever must 2 saves execute at the same time because // this can lead to dirty writes and race conditions + // + // Scenario: auto save was triggered and is currently busy saving to disk. this takes long enough that another auto save + // kicks in. since we never want to trigger 2 saves at the same time, we push out this auto save for the + // configured auto save delay assuming that it can proceed next time it triggers. + // if (this.isBusySaving()) { - diag('doSave(' + versionId + ') - exit - because busy saving', this.resource, new Date()); + diag(`doSave(${versionId}) - exit - because busy saving`, this.resource, new Date()); // Avoid endless loop here and guard if auto save is disabled if (this.autoSaveAfterMilliesEnabled) { @@ -444,7 +458,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil this.lastSaveAttemptTime = Date.now(); // Save to Disk - diag('doSave(' + versionId + ') - before updateContent()', this.resource, new Date()); + diag(`doSave(${versionId}) - before updateContent()`, this.resource, new Date()); this.mapPendingSaveToVersionId[versionId] = this.fileService.updateContent(this.versionOnDiskStat.resource, this.getValue(), { overwriteReadonly: overwriteReadonly, overwriteEncoding: overwriteEncoding, @@ -452,20 +466,20 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil encoding: this.getEncoding(), etag: this.versionOnDiskStat.etag }).then((stat: IFileStat) => { - diag('doSave(' + versionId + ') - after updateContent()', this.resource, new Date()); - - // Telemetry - this.telemetryService.publicLog('filePUT', { mimeType: stat.mime, ext: paths.extname(this.versionOnDiskStat.resource.fsPath) }); + diag(`doSave(${versionId}) - after updateContent()`, this.resource, new Date()); // Remove from pending saves delete this.mapPendingSaveToVersionId[versionId]; + // Telemetry + this.telemetryService.publicLog('filePUT', { mimeType: stat.mime, ext: paths.extname(this.versionOnDiskStat.resource.fsPath) }); + // Update dirty state unless model has changed meanwhile if (versionId === this.versionId) { - diag('doSave(' + versionId + ') - setting dirty to false because versionId did not change', this.resource, new Date()); + diag(`doSave(${versionId}) - setting dirty to false because versionId did not change`, this.resource, new Date()); this.setDirty(false); } else { - diag('doSave(' + versionId + ') - not setting dirty to false because versionId did change meanwhile', this.resource, new Date()); + diag(`doSave(${versionId}) - not setting dirty to false because versionId did change meanwhile`, this.resource, new Date()); } // Updated resolved stat with updated stat, and keep old for event @@ -474,7 +488,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil // Emit File Saved Event this._onDidStateChange.fire(StateChange.SAVED); }, (error) => { - diag('doSave(' + versionId + ') - exit - resulted in a save error: ' + error.toString(), this.resource, new Date()); + diag(`doSave(${versionId}) - exit - resulted in a save error: ${error.toString()}`, this.resource, new Date()); // Remove from pending saves delete this.mapPendingSaveToVersionId[versionId]; @@ -550,10 +564,6 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil TextFileEditorModel.saveErrorHandler.onSaveError(error, this); } - private isBusySaving(): boolean { - return !types.isEmptyObject(this.mapPendingSaveToVersionId); - } - /** * Returns true if the content of this model has changes that are not yet saved back to the disk. */ @@ -600,6 +610,10 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil } } + private isBusySaving(): boolean { + return !types.isEmptyObject(this.mapPendingSaveToVersionId); + } + public getEncoding(): string { return this.preferredEncoding || this.contentEncoding; }