From b618e4a25ec45394f8911922a0f6eaf0b99e238c Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Sat, 20 Jan 2018 14:20:19 +0100 Subject: [PATCH 1/9] introduce writeFileStreamAndFlush --- src/vs/base/node/extfs.ts | 160 ++++++++++++---- src/vs/base/node/pfs.ts | 2 + src/vs/base/test/node/extfs/extfs.test.ts | 220 +++++++++++++++++++++- 3 files changed, 341 insertions(+), 41 deletions(-) diff --git a/src/vs/base/node/extfs.ts b/src/vs/base/node/extfs.ts index b3a4121079efc..b36dd2e9b22ff 100644 --- a/src/vs/base/node/extfs.ts +++ b/src/vs/base/node/extfs.ts @@ -14,6 +14,7 @@ import * as fs from 'fs'; import * as paths from 'path'; import { TPromise } from 'vs/base/common/winjs.base'; import { nfcall } from 'vs/base/common/async'; +import { Readable } from 'stream'; const loop = flow.loop; @@ -54,7 +55,7 @@ export function copy(source: string, target: string, callback: (error: Error) => } if (!stat.isDirectory()) { - return pipeFs(source, target, stat.mode & 511, callback); + return doCopyFile(source, target, stat.mode & 511, callback); } if (copiedSources[source]) { @@ -75,6 +76,38 @@ export function copy(source: string, target: string, callback: (error: Error) => }); } +function doCopyFile(source: string, target: string, mode: number, callback: (error: Error) => void): void { + const reader = fs.createReadStream(source); + const writer = fs.createWriteStream(target, { mode }); + + let finished = false; + const finish = (error?: Error) => { + if (!finished) { + finished = true; + + // in error cases, pass to callback + if (error) { + callback(error); + } + + // we need to explicitly chmod because of https://github.com/nodejs/node/issues/1104 + else { + fs.chmod(target, mode, callback); + } + } + }; + + // handle errors properly + reader.once('error', error => finish(error)); + writer.once('error', error => finish(error)); + + // we are done (underlying fd has been closed) + writer.once('close', () => finish()); + + // start piping + reader.pipe(writer); +} + export function mkdirp(path: string, mode?: number): TPromise { const mkdir = () => nfcall(fs.mkdir, path, mode) .then(null, (err: NodeJS.ErrnoException) => { @@ -88,11 +121,12 @@ export function mkdirp(path: string, mode?: number): TPromise { return TPromise.wrapError(err); }); - // is root? + // stop at root if (path === paths.dirname(path)) { return TPromise.as(true); } + // recursively mkdir return mkdir().then(null, (err: NodeJS.ErrnoException) => { if (err.code === 'ENOENT') { return mkdirp(paths.dirname(path), mode).then(mkdir); @@ -102,40 +136,6 @@ export function mkdirp(path: string, mode?: number): TPromise { }); } -function pipeFs(source: string, target: string, mode: number, callback: (error: Error) => void): void { - let callbackHandled = false; - - const readStream = fs.createReadStream(source); - const writeStream = fs.createWriteStream(target, { mode: mode }); - - const onError = (error: Error) => { - if (!callbackHandled) { - callbackHandled = true; - callback(error); - } - }; - - readStream.on('error', onError); - writeStream.on('error', onError); - - readStream.on('end', () => { - (writeStream).end(() => { // In this case the write stream is known to have an end signature with callback - if (!callbackHandled) { - callbackHandled = true; - - fs.chmod(target, mode, callback); // we need to explicitly chmod because of https://github.com/nodejs/node/issues/1104 - } - }); - }); - - // In node 0.8 there is no easy way to find out when the pipe operation has finished. As such, we use the end property = false - // so that we are in charge of calling end() on the write stream and we will be notified when the write stream is really done. - // We can do this because file streams have an end() method that allows to pass in a callback. - // In node 0.10 there is an event 'finish' emitted from the write stream that can be used. See - // https://groups.google.com/forum/?fromgroups=#!topic/nodejs/YWQ1sRoXOdI - readStream.pipe(writeStream, { end: false }); -} - // Deletes the given path by first moving it out of the workspace. This has two benefits. For one, the operation can return fast because // after the rename, the contents are out of the workspace although not yet deleted. The greater benefit however is that this operation // will fail in case any file is used by another process. fs.unlink() in node will not bail if a file unlinked is used by another process. @@ -320,15 +320,95 @@ export function mv(source: string, target: string, callback: (error: Error) => v }); } +let canFlush = true; +export function writeFileAndFlush(path: string, data: string | NodeBuffer | Readable, options: { mode?: number; flag?: string; }, callback: (error?: Error) => void): void { + options = ensureOptions(options); + + if (data instanceof Readable) { + doWriteFileStreamAndFlush(path, data, options, callback); + } else { + doWriteFileAndFlush(path, data, options, callback); + } +} + +function doWriteFileStreamAndFlush(path: string, reader: Readable, options: { mode?: number; flag?: string; }, callback: (error?: Error) => void): void { + + // finish only once + let finished = false; + const finish = (error?: Error) => { + if (!finished) { + finished = true; + + // in error cases we need to manually close streams + // if the write stream was successfully opened + if (error) { + if (isOpen) { + writer.once('close', () => callback(error)); + writer.close(); + } else { + callback(error); + } + } + + // otherwise just return without error + else { + callback(); + } + } + }; + + // create writer to target + const writer = fs.createWriteStream(path, options); + + // handle errors properly + reader.once('error', error => finish(error)); + writer.once('error', error => finish(error)); + + // save the fd for later use + let fd: number; + let isOpen: boolean; + writer.once('open', descriptor => { + fd = descriptor; + isOpen = true; + }); + + // we are done (underlying fd has been closed) + writer.once('close', () => finish()); + + // handle end event because we are in charge + reader.once('end', () => { + + // flush to disk + if (canFlush && isOpen) { + fs.fdatasync(fd, (syncError: Error) => { + + // In some exotic setups it is well possible that node fails to sync + // In that case we disable flushing and warn to the console + if (syncError) { + console.warn('[node.js fs] fdatasync is now disabled for this session because it failed: ', syncError); + canFlush = false; + } + + writer.end(); + }); + } + + // do not flush + else { + writer.end(); + } + }); + + // end: false means we are in charge of ending the streams properly + reader.pipe(writer, { end: false }); +} + // Calls fs.writeFile() followed by a fs.sync() call to flush the changes to disk // We do this in cases where we want to make sure the data is really on disk and // not in some cache. // // See https://github.com/nodejs/node/blob/v5.10.0/lib/fs.js#L1194 -let canFlush = true; -export function writeFileAndFlush(path: string, data: string | NodeBuffer, options: { mode?: number; flag?: string; }, callback: (error: Error) => void): void { - options = ensureOptions(options); - +function doWriteFileAndFlush(path: string, data: string | NodeBuffer, options: { mode?: number; flag?: string; }, callback: (error?: Error) => void): void { if (!canFlush) { return fs.writeFile(path, data, options, callback); } diff --git a/src/vs/base/node/pfs.ts b/src/vs/base/node/pfs.ts index b949384de1ed1..ba1ab12a7ca65 100644 --- a/src/vs/base/node/pfs.ts +++ b/src/vs/base/node/pfs.ts @@ -13,6 +13,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as platform from 'vs/base/common/platform'; import { once } from 'vs/base/common/event'; +import { Readable } from 'stream'; export function readdir(path: string): TPromise { return nfcall(extfs.readdir, path); @@ -101,6 +102,7 @@ const writeFilePathQueue: { [path: string]: Queue } = Object.create(null); export function writeFile(path: string, data: string, options?: { mode?: number; flag?: string; }): TPromise; export function writeFile(path: string, data: NodeBuffer, options?: { mode?: number; flag?: string; }): TPromise; +export function writeFile(path: string, data: Readable, options?: { mode?: number; flag?: string; }): TPromise; export function writeFile(path: string, data: any, options?: { mode?: number; flag?: string; }): TPromise { let queueKey = toQueueKey(path); diff --git a/src/vs/base/test/node/extfs/extfs.test.ts b/src/vs/base/test/node/extfs/extfs.test.ts index 06ecf0b8ba0b1..2e3e8b848ae87 100644 --- a/src/vs/base/test/node/extfs/extfs.test.ts +++ b/src/vs/base/test/node/extfs/extfs.test.ts @@ -15,6 +15,7 @@ import uuid = require('vs/base/common/uuid'); import strings = require('vs/base/common/strings'); import extfs = require('vs/base/node/extfs'); import { onError } from 'vs/base/test/common/utils'; +import { Readable } from 'stream'; const ignore = () => { }; @@ -22,6 +23,37 @@ const mkdirp = (path: string, mode: number, callback: (error) => void) => { extfs.mkdirp(path, mode).done(() => callback(null), error => callback(error)); }; +const chunkSize = 64 * 1024; +const readError = 'Error while reading'; +function toReadable(value: string, throwError?: boolean): Readable { + const totalChunks = Math.ceil(value.length / chunkSize); + const stringChunks: string[] = []; + + for (let i = 0, j = 0; i < totalChunks; ++i, j += chunkSize) { + stringChunks[i] = value.substr(j, chunkSize); + } + + let counter = 0; + return new Readable({ + read: function () { + if (throwError) { + this.emit('error', new Error(readError)); + } + + let res: string; + let canPush = true; + while (canPush && (res = stringChunks[counter++])) { + canPush = this.push(res); + } + + // EOS + if (!res) { + this.push(null); + } + } + }); +} + suite('Extfs', () => { test('mkdirp', function (done: () => void) { @@ -174,7 +206,7 @@ suite('Extfs', () => { } }); - test('writeFileAndFlush', function (done: () => void) { + test('writeFileAndFlush (string)', function (done: () => void) { const id = uuid.generateUuid(); const parentDir = path.join(os.tmpdir(), 'vsctests', id); const newDir = path.join(parentDir, 'extfs', id); @@ -209,6 +241,192 @@ suite('Extfs', () => { }); }); + test('writeFileAndFlush (stream)', function (done: () => void) { + const id = uuid.generateUuid(); + const parentDir = path.join(os.tmpdir(), 'vsctests', id); + const newDir = path.join(parentDir, 'extfs', id); + const testFile = path.join(newDir, 'flushed.txt'); + + mkdirp(newDir, 493, error => { + if (error) { + return onError(error, done); + } + + assert.ok(fs.existsSync(newDir)); + + extfs.writeFileAndFlush(testFile, toReadable('Hello World'), null, error => { + if (error) { + return onError(error, done); + } + + assert.equal(fs.readFileSync(testFile), 'Hello World'); + + const largeString = (new Array(100 * 1024)).join('Large String\n'); + + extfs.writeFileAndFlush(testFile, toReadable(largeString), null, error => { + if (error) { + return onError(error, done); + } + + assert.equal(fs.readFileSync(testFile), largeString); + + extfs.del(parentDir, os.tmpdir(), done, ignore); + }); + }); + }); + }); + + test('writeFileAndFlush (file stream)', function (done: () => void) { + const id = uuid.generateUuid(); + const parentDir = path.join(os.tmpdir(), 'vsctests', id); + const sourceFile = require.toUrl('./fixtures/index.html'); + const newDir = path.join(parentDir, 'extfs', id); + const testFile = path.join(newDir, 'flushed.txt'); + + mkdirp(newDir, 493, error => { + if (error) { + return onError(error, done); + } + + assert.ok(fs.existsSync(newDir)); + + extfs.writeFileAndFlush(testFile, fs.createReadStream(sourceFile), null, error => { + if (error) { + return onError(error, done); + } + + assert.equal(fs.readFileSync(testFile).toString(), fs.readFileSync(sourceFile).toString()); + + extfs.del(parentDir, os.tmpdir(), done, ignore); + }); + }); + }); + + test('writeFileAndFlush (string, error handling)', function (done: () => void) { + const id = uuid.generateUuid(); + const parentDir = path.join(os.tmpdir(), 'vsctests', id); + const newDir = path.join(parentDir, 'extfs', id); + const testFile = path.join(newDir, 'flushed.txt'); + + mkdirp(newDir, 493, error => { + if (error) { + return onError(error, done); + } + + assert.ok(fs.existsSync(newDir)); + + fs.mkdirSync(testFile); // this will trigger an error because testFile is now a directory! + + extfs.writeFileAndFlush(testFile, 'Hello World', null, error => { + if (!error) { + return onError(new Error('Expected error for writing to readonly file'), done); + } + + extfs.del(parentDir, os.tmpdir(), done, ignore); + }); + }); + }); + + test('writeFileAndFlush (stream, error handling EISDIR)', function (done: () => void) { + const id = uuid.generateUuid(); + const parentDir = path.join(os.tmpdir(), 'vsctests', id); + const newDir = path.join(parentDir, 'extfs', id); + const testFile = path.join(newDir, 'flushed.txt'); + + mkdirp(newDir, 493, error => { + if (error) { + return onError(error, done); + } + + assert.ok(fs.existsSync(newDir)); + + fs.mkdirSync(testFile); // this will trigger an error because testFile is now a directory! + + extfs.writeFileAndFlush(testFile, toReadable('Hello World'), null, error => { + if (!error || (error).code !== 'EISDIR') { + return onError(new Error('Expected EISDIR error for writing to folder but got: ' + (error ? (error).code : 'no error')), done); + } + + extfs.del(parentDir, os.tmpdir(), done, ignore); + }); + }); + }); + + test('writeFileAndFlush (stream, error handling READERROR)', function (done: () => void) { + const id = uuid.generateUuid(); + const parentDir = path.join(os.tmpdir(), 'vsctests', id); + const newDir = path.join(parentDir, 'extfs', id); + const testFile = path.join(newDir, 'flushed.txt'); + + mkdirp(newDir, 493, error => { + if (error) { + return onError(error, done); + } + + assert.ok(fs.existsSync(newDir)); + + extfs.writeFileAndFlush(testFile, toReadable('Hello World', true /* throw error */), null, error => { + if (!error || error.message !== readError) { + return onError(new Error('Expected error for writing to folder'), done); + } + + extfs.del(parentDir, os.tmpdir(), done, ignore); + }); + }); + }); + + test('pasero writeFileAndFlush (stream, error handling EACCES)', function (done: () => void) { + const id = uuid.generateUuid(); + const parentDir = path.join(os.tmpdir(), 'vsctests', id); + const newDir = path.join(parentDir, 'extfs', id); + const testFile = path.join(newDir, 'flushed.txt'); + + mkdirp(newDir, 493, error => { + if (error) { + return onError(error, done); + } + + assert.ok(fs.existsSync(newDir)); + + fs.writeFileSync(testFile, ''); + fs.chmodSync(testFile, 33060); // make readonly + + extfs.writeFileAndFlush(testFile, toReadable('Hello World'), null, error => { + if (!error || !((error).code !== 'EACCES' || (error).code !== 'EPERM')) { + return onError(new Error('Expected EACCES/EPERM error for writing to folder but got: ' + (error ? (error).code : 'no error')), done); + } + + extfs.del(parentDir, os.tmpdir(), done, ignore); + }); + }); + }); + + test('writeFileAndFlush (file stream, error handling)', function (done: () => void) { + const id = uuid.generateUuid(); + const parentDir = path.join(os.tmpdir(), 'vsctests', id); + const sourceFile = require.toUrl('./fixtures/index.html'); + const newDir = path.join(parentDir, 'extfs', id); + const testFile = path.join(newDir, 'flushed.txt'); + + mkdirp(newDir, 493, error => { + if (error) { + return onError(error, done); + } + + assert.ok(fs.existsSync(newDir)); + + fs.mkdirSync(testFile); // this will trigger an error because testFile is now a directory! + + extfs.writeFileAndFlush(testFile, fs.createReadStream(sourceFile), null, error => { + if (!error) { + return onError(new Error('Expected error for writing to folder'), done); + } + + extfs.del(parentDir, os.tmpdir(), done, ignore); + }); + }); + }); + test('writeFileAndFlushSync', function (done: () => void) { const id = uuid.generateUuid(); const parentDir = path.join(os.tmpdir(), 'vsctests', id); From c7da35f1de3d342d12143536a66f43769c9fc9f9 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Sat, 20 Jan 2018 18:10:15 +0100 Subject: [PATCH 2/9] first cut stream support for updateContent --- src/typings/iconv-lite.d.ts | 6 +-- src/vs/base/node/encoding.ts | 10 ++-- src/vs/base/node/extfs.ts | 11 ++-- src/vs/base/node/pfs.ts | 3 +- src/vs/base/test/node/extfs/extfs.test.ts | 3 +- src/vs/platform/files/common/files.ts | 15 +++++- .../common/editor/textEditorModel.ts | 10 ++++ .../files/electron-browser/fileService.ts | 4 +- .../electron-browser/remoteFileService.ts | 9 ++-- .../services/files/node/fileService.ts | 50 ++++++++++++++++--- .../textfile/common/textFileEditorModel.ts | 2 +- .../workbench/test/workbenchTestServices.ts | 4 +- 12 files changed, 94 insertions(+), 33 deletions(-) diff --git a/src/typings/iconv-lite.d.ts b/src/typings/iconv-lite.d.ts index 84c54e320cf37..5ad19bb95b7c3 100644 --- a/src/typings/iconv-lite.d.ts +++ b/src/typings/iconv-lite.d.ts @@ -6,13 +6,13 @@ /// declare module 'iconv-lite' { - export function decode(buffer: NodeBuffer, encoding: string, options?: any): string; + export function decode(buffer: NodeBuffer, encoding: string): string; - export function encode(content: string, encoding: string, options?: any): NodeBuffer; + export function encode(content: string, encoding: string, options?: { addBOM?: boolean }): NodeBuffer; export function encodingExists(encoding: string): boolean; export function decodeStream(encoding: string): NodeJS.ReadWriteStream; - export function encodeStream(encoding: string): NodeJS.ReadWriteStream; + export function encodeStream(encoding: string, options?: { addBOM?: boolean }): NodeJS.ReadWriteStream; } \ No newline at end of file diff --git a/src/vs/base/node/encoding.ts b/src/vs/base/node/encoding.ts index 4177f3ffab5c5..1d134c6576791 100644 --- a/src/vs/base/node/encoding.ts +++ b/src/vs/base/node/encoding.ts @@ -28,11 +28,11 @@ export function bomLength(encoding: string): number { return 0; } -export function decode(buffer: NodeBuffer, encoding: string, options?: any): string { - return iconv.decode(buffer, toNodeEncoding(encoding), options); +export function decode(buffer: NodeBuffer, encoding: string): string { + return iconv.decode(buffer, toNodeEncoding(encoding)); } -export function encode(content: string, encoding: string, options?: any): NodeBuffer { +export function encode(content: string, encoding: string, options?: { addBOM?: boolean }): NodeBuffer { return iconv.encode(content, toNodeEncoding(encoding), options); } @@ -44,6 +44,10 @@ export function decodeStream(encoding: string): NodeJS.ReadWriteStream { return iconv.decodeStream(toNodeEncoding(encoding)); } +export function encodeStream(encoding: string, options?: { addBOM?: boolean }): NodeJS.ReadWriteStream { + return iconv.encodeStream(toNodeEncoding(encoding), options); +} + function toNodeEncoding(enc: string): string { if (enc === UTF8_with_bom) { return UTF8; // iconv does not distinguish UTF 8 with or without BOM, so we need to help it diff --git a/src/vs/base/node/extfs.ts b/src/vs/base/node/extfs.ts index b36dd2e9b22ff..9b540108284e7 100644 --- a/src/vs/base/node/extfs.ts +++ b/src/vs/base/node/extfs.ts @@ -14,7 +14,6 @@ import * as fs from 'fs'; import * as paths from 'path'; import { TPromise } from 'vs/base/common/winjs.base'; import { nfcall } from 'vs/base/common/async'; -import { Readable } from 'stream'; const loop = flow.loop; @@ -321,17 +320,17 @@ export function mv(source: string, target: string, callback: (error: Error) => v } let canFlush = true; -export function writeFileAndFlush(path: string, data: string | NodeBuffer | Readable, options: { mode?: number; flag?: string; }, callback: (error?: Error) => void): void { +export function writeFileAndFlush(path: string, data: string | NodeBuffer | NodeJS.ReadableStream, options: { mode?: number; flag?: string; }, callback: (error?: Error) => void): void { options = ensureOptions(options); - if (data instanceof Readable) { - doWriteFileStreamAndFlush(path, data, options, callback); - } else { + if (typeof data === 'string' || Buffer.isBuffer(data)) { doWriteFileAndFlush(path, data, options, callback); + } else { + doWriteFileStreamAndFlush(path, data, options, callback); } } -function doWriteFileStreamAndFlush(path: string, reader: Readable, options: { mode?: number; flag?: string; }, callback: (error?: Error) => void): void { +function doWriteFileStreamAndFlush(path: string, reader: NodeJS.ReadableStream, options: { mode?: number; flag?: string; }, callback: (error?: Error) => void): void { // finish only once let finished = false; diff --git a/src/vs/base/node/pfs.ts b/src/vs/base/node/pfs.ts index ba1ab12a7ca65..9ebc819fd0236 100644 --- a/src/vs/base/node/pfs.ts +++ b/src/vs/base/node/pfs.ts @@ -13,7 +13,6 @@ import * as fs from 'fs'; import * as os from 'os'; import * as platform from 'vs/base/common/platform'; import { once } from 'vs/base/common/event'; -import { Readable } from 'stream'; export function readdir(path: string): TPromise { return nfcall(extfs.readdir, path); @@ -102,7 +101,7 @@ const writeFilePathQueue: { [path: string]: Queue } = Object.create(null); export function writeFile(path: string, data: string, options?: { mode?: number; flag?: string; }): TPromise; export function writeFile(path: string, data: NodeBuffer, options?: { mode?: number; flag?: string; }): TPromise; -export function writeFile(path: string, data: Readable, options?: { mode?: number; flag?: string; }): TPromise; +export function writeFile(path: string, data: NodeJS.ReadableStream, options?: { mode?: number; flag?: string; }): TPromise; export function writeFile(path: string, data: any, options?: { mode?: number; flag?: string; }): TPromise { let queueKey = toQueueKey(path); diff --git a/src/vs/base/test/node/extfs/extfs.test.ts b/src/vs/base/test/node/extfs/extfs.test.ts index 2e3e8b848ae87..686934d9e0159 100644 --- a/src/vs/base/test/node/extfs/extfs.test.ts +++ b/src/vs/base/test/node/extfs/extfs.test.ts @@ -50,7 +50,8 @@ function toReadable(value: string, throwError?: boolean): Readable { if (!res) { this.push(null); } - } + }, + encoding: 'utf8' }); } diff --git a/src/vs/platform/files/common/files.ts b/src/vs/platform/files/common/files.ts index af89c431ab687..0fe4bf63718e2 100644 --- a/src/vs/platform/files/common/files.ts +++ b/src/vs/platform/files/common/files.ts @@ -83,7 +83,7 @@ export interface IFileService { /** * Updates the content replacing its previous value. */ - updateContent(resource: URI, value: string, options?: IUpdateContentOptions): TPromise; + updateContent(resource: URI, value: string | ITextSnapshot, options?: IUpdateContentOptions): TPromise; /** * Moves the file to a new path identified by the resource. @@ -468,6 +468,19 @@ export interface ITextSnapshot { read(): string; } +/** + * Helper method to convert a snapshot into its full string form. + */ +export function snapshotToString(snapshot: ITextSnapshot): string { + const chunks: string[] = []; + let chunk: string; + while (typeof (chunk = snapshot.read()) === 'string') { + chunks.push(chunk); + } + + return chunks.join(''); +} + /** * Streamable content and meta information of a file. */ diff --git a/src/vs/workbench/common/editor/textEditorModel.ts b/src/vs/workbench/common/editor/textEditorModel.ts index 524ffbdf8d6ec..29550c7197f83 100644 --- a/src/vs/workbench/common/editor/textEditorModel.ts +++ b/src/vs/workbench/common/editor/textEditorModel.ts @@ -13,6 +13,7 @@ import { ITextEditorModel } from 'vs/editor/common/services/resolverService'; import { IModeService } from 'vs/editor/common/services/modeService'; import { IModelService } from 'vs/editor/common/services/modelService'; import { IDisposable } from 'vs/base/common/lifecycle'; +import { ITextSnapshot } from 'vs/platform/files/common/files'; /** * The base text editor model leverages the code editor model. This class is only intended to be subclassed and not instantiated. @@ -142,6 +143,15 @@ export abstract class BaseTextEditorModel extends EditorModel implements ITextEd return null; } + public createSnapshot(): ITextSnapshot { + const model = this.textEditorModel; + if (model) { + return model.createSnapshot(true /* Preserve BOM */); + } + + return null; + } + public isResolved(): boolean { return !!this.textEditorModelHandle; } diff --git a/src/vs/workbench/services/files/electron-browser/fileService.ts b/src/vs/workbench/services/files/electron-browser/fileService.ts index 5eeae64bf3a08..e97cc0ffd1d12 100644 --- a/src/vs/workbench/services/files/electron-browser/fileService.ts +++ b/src/vs/workbench/services/files/electron-browser/fileService.ts @@ -11,7 +11,7 @@ import paths = require('vs/base/common/paths'); import encoding = require('vs/base/node/encoding'); import errors = require('vs/base/common/errors'); import uri from 'vs/base/common/uri'; -import { FileOperation, FileOperationEvent, IFileService, IFilesConfiguration, IResolveFileOptions, IFileStat, IResolveFileResult, IContent, IStreamContent, IImportResult, IResolveContentOptions, IUpdateContentOptions, FileChangesEvent, ICreateFileOptions } from 'vs/platform/files/common/files'; +import { FileOperation, FileOperationEvent, IFileService, IFilesConfiguration, IResolveFileOptions, IFileStat, IResolveFileResult, IContent, IStreamContent, IImportResult, IResolveContentOptions, IUpdateContentOptions, FileChangesEvent, ICreateFileOptions, ITextSnapshot } from 'vs/platform/files/common/files'; import { FileService as NodeFileService, IFileServiceOptions, IEncodingOverride } from 'vs/workbench/services/files/node/fileService'; import { IConfigurationService, IConfigurationChangeEvent } from 'vs/platform/configuration/common/configuration'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; @@ -181,7 +181,7 @@ export class FileService implements IFileService { return this.raw.resolveStreamContent(resource, options); } - public updateContent(resource: uri, value: string, options?: IUpdateContentOptions): TPromise { + public updateContent(resource: uri, value: string | ITextSnapshot, options?: IUpdateContentOptions): TPromise { return this.raw.updateContent(resource, value, options); } diff --git a/src/vs/workbench/services/files/electron-browser/remoteFileService.ts b/src/vs/workbench/services/files/electron-browser/remoteFileService.ts index 2bf1ba014e189..1fb80d9904248 100644 --- a/src/vs/workbench/services/files/electron-browser/remoteFileService.ts +++ b/src/vs/workbench/services/files/electron-browser/remoteFileService.ts @@ -6,7 +6,7 @@ import URI from 'vs/base/common/uri'; import { FileService } from 'vs/workbench/services/files/electron-browser/fileService'; -import { IContent, IStreamContent, IFileStat, IResolveContentOptions, IUpdateContentOptions, IResolveFileOptions, IResolveFileResult, FileOperationEvent, FileOperation, IFileSystemProvider, IStat, FileType, IImportResult, FileChangesEvent, ICreateFileOptions, FileOperationError, FileOperationResult } from 'vs/platform/files/common/files'; +import { IContent, IStreamContent, IFileStat, IResolveContentOptions, IUpdateContentOptions, IResolveFileOptions, IResolveFileResult, FileOperationEvent, FileOperation, IFileSystemProvider, IStat, FileType, IImportResult, FileChangesEvent, ICreateFileOptions, FileOperationError, FileOperationResult, ITextSnapshot, snapshotToString } from 'vs/platform/files/common/files'; import { TPromise } from 'vs/base/common/winjs.base'; import { basename, join } from 'path'; import { IDisposable } from 'vs/base/common/lifecycle'; @@ -351,7 +351,7 @@ export class RemoteFileService extends FileService { } } - updateContent(resource: URI, value: string, options?: IUpdateContentOptions): TPromise { + updateContent(resource: URI, value: string | ITextSnapshot, options?: IUpdateContentOptions): TPromise { if (resource.scheme === Schemas.file) { return super.updateContent(resource, value, options); } else { @@ -361,9 +361,10 @@ export class RemoteFileService extends FileService { } } - private _doUpdateContent(provider: IFileSystemProvider, resource: URI, content: string, options: IUpdateContentOptions): TPromise { + private _doUpdateContent(provider: IFileSystemProvider, resource: URI, content: string | ITextSnapshot, options: IUpdateContentOptions): TPromise { const encoding = this.getEncoding(resource, options.encoding); - return provider.write(resource, encode(content, encoding)).then(() => { + // TODO@Joh support streaming API for remote file system writes + return provider.write(resource, encode(typeof content === 'string' ? content : snapshotToString(content), encoding)).then(() => { return this.resolveFile(resource); }); } diff --git a/src/vs/workbench/services/files/node/fileService.ts b/src/vs/workbench/services/files/node/fileService.ts index b1475b44eb9b9..d55169d2af085 100644 --- a/src/vs/workbench/services/files/node/fileService.ts +++ b/src/vs/workbench/services/files/node/fileService.ts @@ -11,7 +11,7 @@ import os = require('os'); import crypto = require('crypto'); import assert = require('assert'); -import { isParent, FileOperation, FileOperationEvent, IContent, IFileService, IResolveFileOptions, IResolveFileResult, IResolveContentOptions, IFileStat, IStreamContent, FileOperationError, FileOperationResult, IUpdateContentOptions, FileChangeType, IImportResult, FileChangesEvent, ICreateFileOptions, IContentData } from 'vs/platform/files/common/files'; +import { isParent, FileOperation, FileOperationEvent, IContent, IFileService, IResolveFileOptions, IResolveFileResult, IResolveContentOptions, IFileStat, IStreamContent, FileOperationError, FileOperationResult, IUpdateContentOptions, FileChangeType, IImportResult, FileChangesEvent, ICreateFileOptions, IContentData, ITextSnapshot, snapshotToString } from 'vs/platform/files/common/files'; import { MAX_FILE_SIZE } from 'vs/platform/files/node/files'; import { isEqualOrParent } from 'vs/base/common/paths'; import { ResourceMap } from 'vs/base/common/map'; @@ -41,6 +41,7 @@ import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cance import { ILifecycleService, LifecyclePhase } from 'vs/platform/lifecycle/common/lifecycle'; import { getBaseLabel } from 'vs/base/common/labels'; import { assign } from 'vs/base/common/objects'; +import { Readable } from 'stream'; export interface IEncodingOverride { resource: uri; @@ -505,15 +506,16 @@ export class FileService implements IFileService { }); } - public updateContent(resource: uri, value: string, options: IUpdateContentOptions = Object.create(null)): TPromise { + public updateContent(resource: uri, value: string | ITextSnapshot, options: IUpdateContentOptions = Object.create(null)): TPromise { if (this.options.elevationSupport && options.writeElevated) { - return this.doUpdateContentElevated(resource, value, options); + // We can currently only write strings elevated, so we need to convert snapshots properly + return this.doUpdateContentElevated(resource, typeof value === 'string' ? value : snapshotToString(value), options); } return this.doUpdateContent(resource, value, options); } - private doUpdateContent(resource: uri, value: string, options: IUpdateContentOptions = Object.create(null)): TPromise { + private doUpdateContent(resource: uri, value: string | ITextSnapshot, options: IUpdateContentOptions = Object.create(null)): TPromise { const absolutePath = this.toAbsolutePath(resource); // 1.) check file @@ -579,18 +581,25 @@ export class FileService implements IFileService { }); } - private doSetContentsAndResolve(resource: uri, absolutePath: string, value: string, addBOM: boolean, encodingToWrite: string, options?: { mode?: number; flag?: string; }): TPromise { + private doSetContentsAndResolve(resource: uri, absolutePath: string, value: string | ITextSnapshot, addBOM: boolean, encodingToWrite: string, options?: { mode?: number; flag?: string; }): TPromise { let writeFilePromise: TPromise; // Write fast if we do UTF 8 without BOM if (!addBOM && encodingToWrite === encoding.UTF8) { - writeFilePromise = pfs.writeFile(absolutePath, value, options); + if (typeof value === 'string') { + writeFilePromise = pfs.writeFile(absolutePath, value, options); + } else { + writeFilePromise = pfs.writeFile(absolutePath, this.snapshotToReadableStream(value), options); + } } // Otherwise use encoding lib else { - const encoded = encoding.encode(value, encodingToWrite, { addBOM }); - writeFilePromise = pfs.writeFile(absolutePath, encoded, options); + if (typeof value === 'string') { + writeFilePromise = pfs.writeFile(absolutePath, encoding.encode(value, encodingToWrite, { addBOM }), options); + } else { + writeFilePromise = pfs.writeFile(absolutePath, this.snapshotToReadableStream(value).pipe(encoding.encodeStream(encodingToWrite, { addBOM })), options); + } } // set contents @@ -601,6 +610,31 @@ export class FileService implements IFileService { }); } + private snapshotToReadableStream(snapshot: ITextSnapshot): NodeJS.ReadableStream { + return new Readable({ + read: function () { + try { + let chunk: string; + let canPush = true; + + // Push all chunks as long as we can push and as long as + // the underlying snapshot returns strings to us + while (canPush && typeof (chunk = snapshot.read()) === 'string') { + canPush = this.push(chunk); + } + + // Signal EOS by pushing NULL + if (typeof chunk !== 'string') { + this.push(null); + } + } catch (error) { + this.emit('error', error); + } + }, + encoding: encoding.UTF8 // very important, so that strings are passed around and not buffers! + }); + } + private doUpdateContentElevated(resource: uri, value: string, options: IUpdateContentOptions = Object.create(null)): TPromise { const absolutePath = this.toAbsolutePath(resource); diff --git a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts index fc18f6c565646..beb2ea20d7480 100644 --- a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts +++ b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts @@ -702,7 +702,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil // Save to Disk // mark the save operation as currently pending with the versionId (it might have changed from a save participant triggering) diag(`doSave(${versionId}) - before updateContent()`, this.resource, new Date()); - return this.saveSequentializer.setPending(newVersionId, this.fileService.updateContent(this.lastResolvedDiskStat.resource, this.getValue(), { + return this.saveSequentializer.setPending(newVersionId, this.fileService.updateContent(this.lastResolvedDiskStat.resource, this.createSnapshot(), { overwriteReadonly: options.overwriteReadonly, overwriteEncoding: options.overwriteEncoding, mtime: this.lastResolvedDiskStat.mtime, diff --git a/src/vs/workbench/test/workbenchTestServices.ts b/src/vs/workbench/test/workbenchTestServices.ts index e45bc1a4b106b..e6f1b33310f38 100644 --- a/src/vs/workbench/test/workbenchTestServices.ts +++ b/src/vs/workbench/test/workbenchTestServices.ts @@ -33,7 +33,7 @@ import { ServiceCollection } from 'vs/platform/instantiation/common/serviceColle import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService'; import { IEditorGroupService, GroupArrangement, GroupOrientation, IEditorTabOptions, IMoveOptions } from 'vs/workbench/services/group/common/groupService'; import { TextFileService } from 'vs/workbench/services/textfile/common/textFileService'; -import { FileOperationEvent, IFileService, IResolveContentOptions, FileOperationError, IFileStat, IResolveFileResult, IImportResult, FileChangesEvent, IResolveFileOptions, IContent, IUpdateContentOptions, IStreamContent, ICreateFileOptions } from 'vs/platform/files/common/files'; +import { FileOperationEvent, IFileService, IResolveContentOptions, FileOperationError, IFileStat, IResolveFileResult, IImportResult, FileChangesEvent, IResolveFileOptions, IContent, IUpdateContentOptions, IStreamContent, ICreateFileOptions, ITextSnapshot } from 'vs/platform/files/common/files'; import { IModelService } from 'vs/editor/common/services/modelService'; import { ModeServiceImpl } from 'vs/editor/common/services/modeServiceImpl'; import { ModelServiceImpl } from 'vs/editor/common/services/modelServiceImpl'; @@ -755,7 +755,7 @@ export class TestFileService implements IFileService { }); } - updateContent(resource: URI, value: string, options?: IUpdateContentOptions): TPromise { + updateContent(resource: URI, value: string | ITextSnapshot, options?: IUpdateContentOptions): TPromise { return TPromise.timeout(1).then(() => { return { resource, From 34ce394126df4fda096eb4d77015aa44ff4f9d3f Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Sun, 21 Jan 2018 10:43:53 +0100 Subject: [PATCH 3/9] fix fdatasync working properly --- src/vs/base/node/extfs.ts | 44 +++++++++++-------- .../services/files/node/fileService.ts | 8 ++-- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/vs/base/node/extfs.ts b/src/vs/base/node/extfs.ts index 9b540108284e7..963a9a3905e28 100644 --- a/src/vs/base/node/extfs.ts +++ b/src/vs/base/node/extfs.ts @@ -356,14 +356,13 @@ function doWriteFileStreamAndFlush(path: string, reader: NodeJS.ReadableStream, } }; - // create writer to target - const writer = fs.createWriteStream(path, options); + // create writer to target. we set autoClose: false because we want to use the streams + // file descriptor to call fs.fdatasync to ensure the data is flushed to disk + const writer = fs.createWriteStream(path, { mode: options.mode, flags: options.flag, autoClose: false }); - // handle errors properly - reader.once('error', error => finish(error)); - writer.once('error', error => finish(error)); - - // save the fd for later use + // Event: 'open' + // Purpose: save the fd for later use + // Notes: will not be called when there is an error opening the file descriptor! let fd: number; let isOpen: boolean; writer.once('open', descriptor => { @@ -371,11 +370,16 @@ function doWriteFileStreamAndFlush(path: string, reader: NodeJS.ReadableStream, isOpen = true; }); - // we are done (underlying fd has been closed) - writer.once('close', () => finish()); + // Event: 'error' + // Purpose: to return the error to the outside and to close the write stream (does not happen automatically) + reader.once('error', error => finish(error)); + writer.once('error', error => finish(error)); - // handle end event because we are in charge - reader.once('end', () => { + // Event: 'finish' + // Purpose: use fs.fdatasync to flush the contents to disk + // Notes: event is called when the writer has finished writing to the underlying resource. we must call writer.close() + // because we have created the WriteStream with autoClose: false + writer.once('finish', () => { // flush to disk if (canFlush && isOpen) { @@ -388,18 +392,20 @@ function doWriteFileStreamAndFlush(path: string, reader: NodeJS.ReadableStream, canFlush = false; } - writer.end(); + writer.close(); }); - } - - // do not flush - else { - writer.end(); + } else { + writer.close(); } }); - // end: false means we are in charge of ending the streams properly - reader.pipe(writer, { end: false }); + // Event: 'close' + // Purpose: signal we are done to the outside + // Notes: event is called when the writer's filedescriptor is closed + writer.once('close', () => finish()); + + // start data piping + reader.pipe(writer); } // Calls fs.writeFile() followed by a fs.sync() call to flush the changes to disk diff --git a/src/vs/workbench/services/files/node/fileService.ts b/src/vs/workbench/services/files/node/fileService.ts index d55169d2af085..f443fec34a4e0 100644 --- a/src/vs/workbench/services/files/node/fileService.ts +++ b/src/vs/workbench/services/files/node/fileService.ts @@ -10,8 +10,7 @@ import fs = require('fs'); import os = require('os'); import crypto = require('crypto'); import assert = require('assert'); - -import { isParent, FileOperation, FileOperationEvent, IContent, IFileService, IResolveFileOptions, IResolveFileResult, IResolveContentOptions, IFileStat, IStreamContent, FileOperationError, FileOperationResult, IUpdateContentOptions, FileChangeType, IImportResult, FileChangesEvent, ICreateFileOptions, IContentData, ITextSnapshot, snapshotToString } from 'vs/platform/files/common/files'; +import { isParent, FileOperation, FileOperationEvent, IContent, IFileService, IResolveFileOptions, IResolveFileResult, IResolveContentOptions, IFileStat, IStreamContent, FileOperationError, FileOperationResult, IUpdateContentOptions, FileChangeType, IImportResult, FileChangesEvent, ICreateFileOptions, IContentData, ITextSnapshot } from 'vs/platform/files/common/files'; import { MAX_FILE_SIZE } from 'vs/platform/files/node/files'; import { isEqualOrParent } from 'vs/base/common/paths'; import { ResourceMap } from 'vs/base/common/map'; @@ -508,8 +507,7 @@ export class FileService implements IFileService { public updateContent(resource: uri, value: string | ITextSnapshot, options: IUpdateContentOptions = Object.create(null)): TPromise { if (this.options.elevationSupport && options.writeElevated) { - // We can currently only write strings elevated, so we need to convert snapshots properly - return this.doUpdateContentElevated(resource, typeof value === 'string' ? value : snapshotToString(value), options); + return this.doUpdateContentElevated(resource, value, options); } return this.doUpdateContent(resource, value, options); @@ -635,7 +633,7 @@ export class FileService implements IFileService { }); } - private doUpdateContentElevated(resource: uri, value: string, options: IUpdateContentOptions = Object.create(null)): TPromise { + private doUpdateContentElevated(resource: uri, value: string | ITextSnapshot, options: IUpdateContentOptions = Object.create(null)): TPromise { const absolutePath = this.toAbsolutePath(resource); // 1.) check file From 1d56aabed51fc8324144e30289dab18b4ec5a8cd Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Sun, 21 Jan 2018 10:51:57 +0100 Subject: [PATCH 4/9] add more text snapshot tests --- .../files/test/node/fileService.test.ts | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/src/vs/workbench/services/files/test/node/fileService.test.ts b/src/vs/workbench/services/files/test/node/fileService.test.ts index 3a97a2a5cecac..863f43f872129 100644 --- a/src/vs/workbench/services/files/test/node/fileService.test.ts +++ b/src/vs/workbench/services/files/test/node/fileService.test.ts @@ -22,6 +22,7 @@ import { onError } from 'vs/base/test/common/utils'; import { TestContextService, TestTextResourceConfigurationService, getRandomTestPath, TestLifecycleService } from 'vs/workbench/test/workbenchTestServices'; import { Workspace, toWorkspaceFolders } from 'vs/platform/workspace/common/workspace'; import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; +import { TextModel } from 'vs/editor/common/model/textModel'; suite('FileService', () => { let service: FileService; @@ -581,6 +582,54 @@ suite('FileService', () => { }, error => onError(error, done)); }); + test('updateContent (ITextSnapShot)', function (done: () => void) { + const resource = uri.file(path.join(testDir, 'small.txt')); + + service.resolveContent(resource).done(c => { + assert.equal(c.value, 'Small File'); + + const model = TextModel.createFromString('Updates to the small file'); + + return service.updateContent(c.resource, model.createSnapshot()).then(c => { + assert.equal(fs.readFileSync(resource.fsPath), 'Updates to the small file'); + + model.dispose(); + + done(); + }); + }, error => onError(error, done)); + }); + + test('updateContent (large file)', function (done: () => void) { + const resource = uri.file(path.join(testDir, 'lorem.txt')); + + service.resolveContent(resource).done(c => { + const newValue = c.value + c.value; + c.value = newValue; + + return service.updateContent(c.resource, c.value).then(c => { + assert.equal(fs.readFileSync(resource.fsPath), newValue); + + done(); + }); + }, error => onError(error, done)); + }); + + test('updateContent (large file, ITextSnapShot)', function (done: () => void) { + const resource = uri.file(path.join(testDir, 'lorem.txt')); + + service.resolveContent(resource).done(c => { + const newValue = c.value + c.value; + const model = TextModel.createFromString(newValue); + + return service.updateContent(c.resource, model.createSnapshot()).then(c => { + assert.equal(fs.readFileSync(resource.fsPath), newValue); + + done(); + }); + }, error => onError(error, done)); + }); + test('updateContent - use encoding (UTF 16 BE)', function (done: () => void) { const resource = uri.file(path.join(testDir, 'small.txt')); const encoding = 'utf16be'; @@ -602,6 +651,31 @@ suite('FileService', () => { }, error => onError(error, done)); }); + test('updateContent - use encoding (UTF 16 BE, ITextSnapShot)', function (done: () => void) { + const resource = uri.file(path.join(testDir, 'small.txt')); + const encoding = 'utf16be'; + + service.resolveContent(resource).done(c => { + c.encoding = encoding; + + const model = TextModel.createFromString(c.value); + + return service.updateContent(c.resource, model.createSnapshot(), { encoding: encoding }).then(c => { + return encodingLib.detectEncodingByBOM(c.resource.fsPath).then((enc) => { + assert.equal(enc, encodingLib.UTF16be); + + return service.resolveContent(resource).then(c => { + assert.equal(c.encoding, encoding); + + model.dispose(); + + done(); + }); + }); + }); + }, error => onError(error, done)); + }); + test('updateContent - encoding preserved (UTF 16 LE)', function (done: () => void) { const encoding = 'utf16le'; const resource = uri.file(path.join(testDir, 'some_utf16le.css')); @@ -625,6 +699,31 @@ suite('FileService', () => { }, error => onError(error, done)); }); + test('updateContent - encoding preserved (UTF 16 LE, ITextSnapShot)', function (done: () => void) { + const encoding = 'utf16le'; + const resource = uri.file(path.join(testDir, 'some_utf16le.css')); + + service.resolveContent(resource).done(c => { + assert.equal(c.encoding, encoding); + + const model = TextModel.createFromString('Some updates'); + + return service.updateContent(c.resource, model.createSnapshot(), { encoding: encoding }).then(c => { + return encodingLib.detectEncodingByBOM(c.resource.fsPath).then((enc) => { + assert.equal(enc, encodingLib.UTF16le); + + return service.resolveContent(resource).then(c => { + assert.equal(c.encoding, encoding); + + model.dispose(); + + done(); + }); + }); + }); + }, error => onError(error, done)); + }); + test('resolveContent - large file', function (done: () => void) { const resource = uri.file(path.join(testDir, 'lorem.txt')); From d44201fa07b1c89ac35875f90bdc13b9b68e3b3c Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Sun, 21 Jan 2018 12:20:46 +0100 Subject: [PATCH 5/9] wire in ITextSnapshot for backup writing --- src/vs/platform/files/common/files.ts | 26 ++++++++++ .../parts/backup/common/backupModelTracker.ts | 4 +- .../files/electron-browser/fileActions.ts | 2 +- .../electron-browser/views/explorerViewer.ts | 2 +- .../services/backup/common/backup.ts | 6 +-- .../services/backup/node/backupFileService.ts | 22 +++++--- .../test/node/backupFileService.test.ts | 52 ++++++++++++++++++- .../files/test/node/fileService.test.ts | 14 +++-- .../textfile/common/textFileService.ts | 4 +- .../services/textfile/common/textfiles.ts | 4 +- .../workbench/test/workbenchTestServices.ts | 2 +- 11 files changed, 116 insertions(+), 22 deletions(-) diff --git a/src/vs/platform/files/common/files.ts b/src/vs/platform/files/common/files.ts index 0fe4bf63718e2..9f313e5a55ae4 100644 --- a/src/vs/platform/files/common/files.ts +++ b/src/vs/platform/files/common/files.ts @@ -481,6 +481,32 @@ export function snapshotToString(snapshot: ITextSnapshot): string { return chunks.join(''); } +/** + * Helper that wraps around a ITextSnapshot and allows to have a + * preamble that the read() method will return first. + */ +export class BufferedTextSnapshot implements ITextSnapshot { + private preambleHandled: boolean; + + constructor(private snapshot: ITextSnapshot, private preamble: string) { + } + + public read(): string { + let value = this.snapshot.read(); + if (!this.preambleHandled) { + this.preambleHandled = true; + + if (typeof value === 'string') { + value = this.preamble + value; + } else { + value = this.preamble; + } + } + + return value; + } +} + /** * Streamable content and meta information of a file. */ diff --git a/src/vs/workbench/parts/backup/common/backupModelTracker.ts b/src/vs/workbench/parts/backup/common/backupModelTracker.ts index 7078c5cd4b565..1c61b9715e28e 100644 --- a/src/vs/workbench/parts/backup/common/backupModelTracker.ts +++ b/src/vs/workbench/parts/backup/common/backupModelTracker.ts @@ -72,14 +72,14 @@ export class BackupModelTracker implements IWorkbenchContribution { // Do not backup when auto save after delay is configured if (!this.configuredAutoSaveAfterDelay) { const model = this.textFileService.models.get(event.resource); - this.backupFileService.backupResource(model.getResource(), model.getValue(), model.getVersionId()).done(null, errors.onUnexpectedError); + this.backupFileService.backupResource(model.getResource(), model.createSnapshot(), model.getVersionId()).done(null, errors.onUnexpectedError); } } } private onUntitledModelChanged(resource: Uri): void { if (this.untitledEditorService.isDirty(resource)) { - this.untitledEditorService.loadOrCreate({ resource }).then(model => this.backupFileService.backupResource(resource, model.getValue(), model.getVersionId())).done(null, errors.onUnexpectedError); + this.untitledEditorService.loadOrCreate({ resource }).then(model => this.backupFileService.backupResource(resource, model.createSnapshot(), model.getVersionId())).done(null, errors.onUnexpectedError); } else { this.discardBackup(resource); } diff --git a/src/vs/workbench/parts/files/electron-browser/fileActions.ts b/src/vs/workbench/parts/files/electron-browser/fileActions.ts index e80a6804eb1f0..952968f7641f2 100644 --- a/src/vs/workbench/parts/files/electron-browser/fileActions.ts +++ b/src/vs/workbench/parts/files/electron-browser/fileActions.ts @@ -318,7 +318,7 @@ class RenameFileAction extends BaseRenameAction { const model = this.textFileService.models.get(d); - return this.backupFileService.backupResource(renamed, model.getValue(), model.getVersionId()); + return this.backupFileService.backupResource(renamed, model.createSnapshot(), model.getVersionId()); })) // 2. soft revert all dirty since we have backed up their contents diff --git a/src/vs/workbench/parts/files/electron-browser/views/explorerViewer.ts b/src/vs/workbench/parts/files/electron-browser/views/explorerViewer.ts index a812d54aaff8d..49550e507a130 100644 --- a/src/vs/workbench/parts/files/electron-browser/views/explorerViewer.ts +++ b/src/vs/workbench/parts/files/electron-browser/views/explorerViewer.ts @@ -968,7 +968,7 @@ export class FileDragAndDrop extends SimpleFileResourceDragAndDrop { const model = this.textFileService.models.get(d); - return this.backupFileService.backupResource(moved, model.getValue(), model.getVersionId()); + return this.backupFileService.backupResource(moved, model.createSnapshot(), model.getVersionId()); })) // 2. soft revert all dirty since we have backed up their contents diff --git a/src/vs/workbench/services/backup/common/backup.ts b/src/vs/workbench/services/backup/common/backup.ts index 7a46ffa491e76..0b4d35f5d0cdb 100644 --- a/src/vs/workbench/services/backup/common/backup.ts +++ b/src/vs/workbench/services/backup/common/backup.ts @@ -8,7 +8,7 @@ import Uri from 'vs/base/common/uri'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; import { TPromise } from 'vs/base/common/winjs.base'; -import { IResolveContentOptions, IUpdateContentOptions } from 'vs/platform/files/common/files'; +import { IResolveContentOptions, IUpdateContentOptions, ITextSnapshot } from 'vs/platform/files/common/files'; import { ITextBufferFactory } from 'vs/editor/common/model'; export const IBackupFileService = createDecorator('backupFileService'); @@ -52,10 +52,10 @@ export interface IBackupFileService { * Backs up a resource. * * @param resource The resource to back up. - * @param content The content of the resource. + * @param content The content of the resource as value or snapshot. * @param versionId The version id of the resource to backup. */ - backupResource(resource: Uri, content: string, versionId?: number): TPromise; + backupResource(resource: Uri, content: string | ITextSnapshot, versionId?: number): TPromise; /** * Gets a list of file backups for the current workspace. diff --git a/src/vs/workbench/services/backup/node/backupFileService.ts b/src/vs/workbench/services/backup/node/backupFileService.ts index ff3543726ce70..3ef38e18dc5cf 100644 --- a/src/vs/workbench/services/backup/node/backupFileService.ts +++ b/src/vs/workbench/services/backup/node/backupFileService.ts @@ -11,7 +11,7 @@ import * as pfs from 'vs/base/node/pfs'; import Uri from 'vs/base/common/uri'; import { ResourceQueue } from 'vs/base/common/async'; import { IBackupFileService, BACKUP_FILE_UPDATE_OPTIONS } from 'vs/workbench/services/backup/common/backup'; -import { IFileService } from 'vs/platform/files/common/files'; +import { IFileService, ITextSnapshot, BufferedTextSnapshot, IFileStat } from 'vs/platform/files/common/files'; import { TPromise } from 'vs/base/common/winjs.base'; import { readToMatchingString } from 'vs/base/node/stream'; import { Range } from 'vs/editor/common/core/range'; @@ -149,7 +149,7 @@ export class BackupFileService implements IBackupFileService { }); } - public backupResource(resource: Uri, content: string, versionId?: number): TPromise { + public backupResource(resource: Uri, content: string | ITextSnapshot, versionId?: number): TPromise { if (this.isShuttingDown) { return TPromise.as(void 0); } @@ -164,11 +164,21 @@ export class BackupFileService implements IBackupFileService { return void 0; // return early if backup version id matches requested one } - // Add metadata to top of file - content = `${resource.toString()}${BackupFileService.META_MARKER}${content}`; - return this.ioOperationQueues.queueFor(backupResource).queue(() => { - return this.fileService.updateContent(backupResource, content, BACKUP_FILE_UPDATE_OPTIONS).then(() => model.add(backupResource, versionId)); + const preamble = `${resource.toString()}${BackupFileService.META_MARKER}`; + + // Update content with value + let updateContentPromise: TPromise; + if (typeof content === 'string') { + updateContentPromise = this.fileService.updateContent(backupResource, `${preamble}${content}`, BACKUP_FILE_UPDATE_OPTIONS); + } + + // Update content with snapshot + else { + updateContentPromise = this.fileService.updateContent(backupResource, new BufferedTextSnapshot(content, preamble), BACKUP_FILE_UPDATE_OPTIONS); + } + + return updateContentPromise.then(() => model.add(backupResource, versionId)); }); }); } diff --git a/src/vs/workbench/services/backup/test/node/backupFileService.test.ts b/src/vs/workbench/services/backup/test/node/backupFileService.test.ts index 84ae8794ec775..2720754abbbf6 100644 --- a/src/vs/workbench/services/backup/test/node/backupFileService.test.ts +++ b/src/vs/workbench/services/backup/test/node/backupFileService.test.ts @@ -16,7 +16,7 @@ import pfs = require('vs/base/node/pfs'); import Uri from 'vs/base/common/uri'; import { BackupFileService, BackupFilesModel } from 'vs/workbench/services/backup/node/backupFileService'; import { FileService } from 'vs/workbench/services/files/node/fileService'; -import { createTextBufferFactory } from 'vs/editor/common/model/textModel'; +import { createTextBufferFactory, TextModel } from 'vs/editor/common/model/textModel'; import { TestContextService, TestTextResourceConfigurationService, getRandomTestPath, TestLifecycleService } from 'vs/workbench/test/workbenchTestServices'; import { Workspace, toWorkspaceFolders } from 'vs/platform/workspace/common/workspace'; import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; @@ -121,6 +121,56 @@ suite('BackupFileService', () => { done(); }); }); + + test('text file (ITextSnapshot)', function (done: () => void) { + const model = TextModel.createFromString('test'); + + service.backupResource(fooFile, model.createSnapshot()).then(() => { + assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'file')).length, 1); + assert.equal(fs.existsSync(fooBackupPath), true); + assert.equal(fs.readFileSync(fooBackupPath), `${fooFile.toString()}\ntest`); + model.dispose(); + done(); + }); + }); + + test('untitled file (ITextSnapshot)', function (done: () => void) { + const model = TextModel.createFromString('test'); + + service.backupResource(untitledFile, model.createSnapshot()).then(() => { + assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'untitled')).length, 1); + assert.equal(fs.existsSync(untitledBackupPath), true); + assert.equal(fs.readFileSync(untitledBackupPath), `${untitledFile.toString()}\ntest`); + model.dispose(); + done(); + }); + }); + + test('text file (large file, ITextSnapshot)', function (done: () => void) { + const largeString = (new Array(100 * 1024)).join('Large String\n'); + const model = TextModel.createFromString(largeString); + + service.backupResource(fooFile, model.createSnapshot()).then(() => { + assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'file')).length, 1); + assert.equal(fs.existsSync(fooBackupPath), true); + assert.equal(fs.readFileSync(fooBackupPath), `${fooFile.toString()}\n${largeString}`); + model.dispose(); + done(); + }); + }); + + test('untitled file (large file, ITextSnapshot)', function (done: () => void) { + const largeString = (new Array(100 * 1024)).join('Large String\n'); + const model = TextModel.createFromString(largeString); + + service.backupResource(untitledFile, model.createSnapshot()).then(() => { + assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'untitled')).length, 1); + assert.equal(fs.existsSync(untitledBackupPath), true); + assert.equal(fs.readFileSync(untitledBackupPath), `${untitledFile.toString()}\n${largeString}`); + model.dispose(); + done(); + }); + }); }); suite('discardResourceBackup', () => { diff --git a/src/vs/workbench/services/files/test/node/fileService.test.ts b/src/vs/workbench/services/files/test/node/fileService.test.ts index 863f43f872129..378b1b91a08d1 100644 --- a/src/vs/workbench/services/files/test/node/fileService.test.ts +++ b/src/vs/workbench/services/files/test/node/fileService.test.ts @@ -945,26 +945,32 @@ suite('FileService', () => { fs.readFile(resource.fsPath, (error, data) => { assert.equal(encodingLib.detectEncodingByBOMFromBuffer(data, 512), null); + const model = TextModel.createFromString('Hello Bom'); + // Update content: UTF_8 => UTF_8_BOM - _service.updateContent(resource, 'Hello Bom', { encoding: encodingLib.UTF8_with_bom }).done(() => { + _service.updateContent(resource, model.createSnapshot(), { encoding: encodingLib.UTF8_with_bom }).done(() => { fs.readFile(resource.fsPath, (error, data) => { assert.equal(encodingLib.detectEncodingByBOMFromBuffer(data, 512), encodingLib.UTF8); // Update content: PRESERVE BOM when using UTF-8 - _service.updateContent(resource, 'Please stay Bom', { encoding: encodingLib.UTF8 }).done(() => { + model.setValue('Please stay Bom'); + _service.updateContent(resource, model.createSnapshot(), { encoding: encodingLib.UTF8 }).done(() => { fs.readFile(resource.fsPath, (error, data) => { assert.equal(encodingLib.detectEncodingByBOMFromBuffer(data, 512), encodingLib.UTF8); // Update content: REMOVE BOM - _service.updateContent(resource, 'Go away Bom', { encoding: encodingLib.UTF8, overwriteEncoding: true }).done(() => { + model.setValue('Go away Bom'); + _service.updateContent(resource, model.createSnapshot(), { encoding: encodingLib.UTF8, overwriteEncoding: true }).done(() => { fs.readFile(resource.fsPath, (error, data) => { assert.equal(encodingLib.detectEncodingByBOMFromBuffer(data, 512), null); // Update content: BOM comes not back - _service.updateContent(resource, 'Do not come back Bom', { encoding: encodingLib.UTF8 }).done(() => { + model.setValue('Do not come back Bom'); + _service.updateContent(resource, model.createSnapshot(), { encoding: encodingLib.UTF8 }).done(() => { fs.readFile(resource.fsPath, (error, data) => { assert.equal(encodingLib.detectEncodingByBOMFromBuffer(data, 512), null); + model.dispose(); _service.dispose(); done(); }); diff --git a/src/vs/workbench/services/textfile/common/textFileService.ts b/src/vs/workbench/services/textfile/common/textFileService.ts index 6e5bf84bf4a63..a1ef38dd364e6 100644 --- a/src/vs/workbench/services/textfile/common/textFileService.ts +++ b/src/vs/workbench/services/textfile/common/textFileService.ts @@ -241,7 +241,7 @@ export abstract class TextFileService implements ITextFileService { private doBackupAll(dirtyFileModels: ITextFileEditorModel[], untitledResources: URI[]): TPromise { // Handle file resources first - return TPromise.join(dirtyFileModels.map(model => this.backupFileService.backupResource(model.getResource(), model.getValue(), model.getVersionId()))).then(results => { + return TPromise.join(dirtyFileModels.map(model => this.backupFileService.backupResource(model.getResource(), model.createSnapshot(), model.getVersionId()))).then(results => { // Handle untitled resources const untitledModelPromises = untitledResources @@ -250,7 +250,7 @@ export abstract class TextFileService implements ITextFileService { return TPromise.join(untitledModelPromises).then(untitledModels => { const untitledBackupPromises = untitledModels.map(model => { - return this.backupFileService.backupResource(model.getResource(), model.getValue(), model.getVersionId()); + return this.backupFileService.backupResource(model.getResource(), model.createSnapshot(), model.getVersionId()); }); return TPromise.join(untitledBackupPromises).then(() => void 0); diff --git a/src/vs/workbench/services/textfile/common/textfiles.ts b/src/vs/workbench/services/textfile/common/textfiles.ts index 4be8ffb329e92..6000c9734e52e 100644 --- a/src/vs/workbench/services/textfile/common/textfiles.ts +++ b/src/vs/workbench/services/textfile/common/textfiles.ts @@ -9,7 +9,7 @@ import URI from 'vs/base/common/uri'; import Event from 'vs/base/common/event'; import { IDisposable } from 'vs/base/common/lifecycle'; import { IEncodingSupport, ConfirmResult } from 'vs/workbench/common/editor'; -import { IBaseStat, IResolveContentOptions } from 'vs/platform/files/common/files'; +import { IBaseStat, IResolveContentOptions, ITextSnapshot } from 'vs/platform/files/common/files'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; import { ITextEditorModel } from 'vs/editor/common/services/resolverService'; import { ITextBufferFactory } from 'vs/editor/common/model'; @@ -202,6 +202,8 @@ export interface ITextFileEditorModel extends ITextEditorModel, IEncodingSupport getValue(): string; + createSnapshot(): ITextSnapshot; + isDirty(): boolean; isResolved(): boolean; diff --git a/src/vs/workbench/test/workbenchTestServices.ts b/src/vs/workbench/test/workbenchTestServices.ts index e6f1b33310f38..50f9c781ff861 100644 --- a/src/vs/workbench/test/workbenchTestServices.ts +++ b/src/vs/workbench/test/workbenchTestServices.ts @@ -856,7 +856,7 @@ export class TestBackupFileService implements IBackupFileService { return null; } - public backupResource(resource: URI, content: string): TPromise { + public backupResource(resource: URI, content: string | ITextSnapshot): TPromise { return TPromise.as(void 0); } From fbc94d2a8f61a7bd3a770d2a6d02ce7efe41d7b3 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Sun, 21 Jan 2018 12:43:18 +0100 Subject: [PATCH 6/9] snapshot support for getFirstLineText --- .../workbench/common/editor/textEditorModel.ts | 16 +++++++++++++--- .../common/editor/untitledEditorModel.ts | 9 --------- .../textfile/common/textFileEditorModel.ts | 2 +- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/common/editor/textEditorModel.ts b/src/vs/workbench/common/editor/textEditorModel.ts index 29550c7197f83..8df2f87b499e0 100644 --- a/src/vs/workbench/common/editor/textEditorModel.ts +++ b/src/vs/workbench/common/editor/textEditorModel.ts @@ -91,7 +91,9 @@ export abstract class BaseTextEditorModel extends EditorModel implements ITextEd return this; } - protected getFirstLineText(value: string | ITextBufferFactory): string { + protected getFirstLineText(value: string | ITextBufferFactory | ITextSnapshot): string { + + // string if (typeof value === 'string') { const firstLineText = value.substr(0, 100); @@ -106,9 +108,17 @@ export abstract class BaseTextEditorModel extends EditorModel implements ITextEd } return firstLineText.substr(0, Math.min(crIndex, lfIndex)); - } else { - return value.getFirstLineText(100); } + + // text buffer factory + const textBufferFactory = value as ITextBufferFactory; + if (typeof textBufferFactory.getFirstLineText === 'function') { + return textBufferFactory.getFirstLineText(100); + } + + // text snapshot + const textSnapshot = value as ITextSnapshot; + return this.getFirstLineText(textSnapshot.read() || ''); } /** diff --git a/src/vs/workbench/common/editor/untitledEditorModel.ts b/src/vs/workbench/common/editor/untitledEditorModel.ts index 9a817653721ae..c7dcee066bd45 100644 --- a/src/vs/workbench/common/editor/untitledEditorModel.ts +++ b/src/vs/workbench/common/editor/untitledEditorModel.ts @@ -10,7 +10,6 @@ import { IEncodingSupport } from 'vs/workbench/common/editor'; import { BaseTextEditorModel } from 'vs/workbench/common/editor/textEditorModel'; import URI from 'vs/base/common/uri'; import { PLAINTEXT_MODE_ID } from 'vs/editor/common/modes/modesRegistry'; -import { EndOfLinePreference } from 'vs/editor/common/model'; import { CONTENT_CHANGE_EVENT_BUFFER_DELAY } from 'vs/platform/files/common/files'; import { IModeService } from 'vs/editor/common/services/modeService'; import { IModelService } from 'vs/editor/common/services/modelService'; @@ -113,14 +112,6 @@ export class UntitledEditorModel extends BaseTextEditorModel implements IEncodin return this.versionId; } - public getValue(): string { - if (this.textEditorModel) { - return this.textEditorModel.getValue(EndOfLinePreference.TextDefined, true /* Preserve BOM */); - } - - return null; - } - public getModeId(): string { if (this.textEditorModel) { return this.textEditorModel.getLanguageIdentifier().language; diff --git a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts index beb2ea20d7480..88197be4a82f5 100644 --- a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts +++ b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts @@ -188,7 +188,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil return; } - const firstLineText = this.getFirstLineText(this.textEditorModel.getValue()); + const firstLineText = this.getFirstLineText(this.textEditorModel.createSnapshot()); const mode = this.getOrCreateMode(this.modeService, modeId, firstLineText); this.modelService.setMode(this.textEditorModel, mode); From 0628d026f004687134a54ce1fb55f593bcac1fd4 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Sun, 21 Jan 2018 13:26:15 +0100 Subject: [PATCH 7/9] move BackupSnapshot around --- src/vs/platform/files/common/files.ts | 26 ------------------- .../services/backup/node/backupFileService.ts | 26 +++++++++++++++++-- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/src/vs/platform/files/common/files.ts b/src/vs/platform/files/common/files.ts index 9f313e5a55ae4..0fe4bf63718e2 100644 --- a/src/vs/platform/files/common/files.ts +++ b/src/vs/platform/files/common/files.ts @@ -481,32 +481,6 @@ export function snapshotToString(snapshot: ITextSnapshot): string { return chunks.join(''); } -/** - * Helper that wraps around a ITextSnapshot and allows to have a - * preamble that the read() method will return first. - */ -export class BufferedTextSnapshot implements ITextSnapshot { - private preambleHandled: boolean; - - constructor(private snapshot: ITextSnapshot, private preamble: string) { - } - - public read(): string { - let value = this.snapshot.read(); - if (!this.preambleHandled) { - this.preambleHandled = true; - - if (typeof value === 'string') { - value = this.preamble + value; - } else { - value = this.preamble; - } - } - - return value; - } -} - /** * Streamable content and meta information of a file. */ diff --git a/src/vs/workbench/services/backup/node/backupFileService.ts b/src/vs/workbench/services/backup/node/backupFileService.ts index 3ef38e18dc5cf..18c4e2a7b5073 100644 --- a/src/vs/workbench/services/backup/node/backupFileService.ts +++ b/src/vs/workbench/services/backup/node/backupFileService.ts @@ -11,7 +11,7 @@ import * as pfs from 'vs/base/node/pfs'; import Uri from 'vs/base/common/uri'; import { ResourceQueue } from 'vs/base/common/async'; import { IBackupFileService, BACKUP_FILE_UPDATE_OPTIONS } from 'vs/workbench/services/backup/common/backup'; -import { IFileService, ITextSnapshot, BufferedTextSnapshot, IFileStat } from 'vs/platform/files/common/files'; +import { IFileService, ITextSnapshot, IFileStat } from 'vs/platform/files/common/files'; import { TPromise } from 'vs/base/common/winjs.base'; import { readToMatchingString } from 'vs/base/node/stream'; import { Range } from 'vs/editor/common/core/range'; @@ -28,6 +28,28 @@ export interface IBackupFilesModel { clear(): void; } +export class BackupSnapshot implements ITextSnapshot { + private preambleHandled: boolean; + + constructor(private snapshot: ITextSnapshot, private preamble: string) { + } + + public read(): string { + let value = this.snapshot.read(); + if (!this.preambleHandled) { + this.preambleHandled = true; + + if (typeof value === 'string') { + value = this.preamble + value; + } else { + value = this.preamble; + } + } + + return value; + } +} + export class BackupFilesModel implements IBackupFilesModel { private cache: { [resource: string]: number /* version ID */ } = Object.create(null); @@ -175,7 +197,7 @@ export class BackupFileService implements IBackupFileService { // Update content with snapshot else { - updateContentPromise = this.fileService.updateContent(backupResource, new BufferedTextSnapshot(content, preamble), BACKUP_FILE_UPDATE_OPTIONS); + updateContentPromise = this.fileService.updateContent(backupResource, new BackupSnapshot(content, preamble), BACKUP_FILE_UPDATE_OPTIONS); } return updateContentPromise.then(() => model.add(backupResource, versionId)); From 56b067cfdf8d24d93a3261da639bc44106d90fed Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 22 Jan 2018 16:10:00 +0100 Subject: [PATCH 8/9] adopt more snapshots --- src/vs/editor/common/model/textModel.ts | 11 +++++++++++ .../parts/files/electron-browser/saveErrorHandler.ts | 5 +++-- .../services/textfile/common/textFileService.ts | 7 +++++-- .../textfile/electron-browser/textFileService.ts | 4 +++- src/vs/workbench/test/workbenchTestServices.ts | 5 +++-- 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/vs/editor/common/model/textModel.ts b/src/vs/editor/common/model/textModel.ts index 90e95ac7bb7f4..27c58c24d2e62 100644 --- a/src/vs/editor/common/model/textModel.ts +++ b/src/vs/editor/common/model/textModel.ts @@ -82,6 +82,17 @@ export function createTextBufferFactoryFromStream(stream: IStringStream): TPromi }); } +export function createTextBufferFactoryFromSnapshot(snapshot: ITextSnapshot): model.ITextBufferFactory { + let builder = createTextBufferBuilder(); + + let chunk: string; + while (typeof (chunk = snapshot.read()) === 'string') { + builder.acceptChunk(chunk); + } + + return builder.finish(); +} + export function createTextBuffer(value: string | model.ITextBufferFactory, defaultEOL: model.DefaultEndOfLine): model.ITextBuffer { const factory = (typeof value === 'string' ? createTextBufferFactory(value) : value); return factory.create(defaultEOL); diff --git a/src/vs/workbench/parts/files/electron-browser/saveErrorHandler.ts b/src/vs/workbench/parts/files/electron-browser/saveErrorHandler.ts index 379a2a53d81bc..3a0864e2ea7d5 100644 --- a/src/vs/workbench/parts/files/electron-browser/saveErrorHandler.ts +++ b/src/vs/workbench/parts/files/electron-browser/saveErrorHandler.ts @@ -31,6 +31,7 @@ import { FileEditorInput } from 'vs/workbench/parts/files/common/editors/fileEdi import { IModelService } from 'vs/editor/common/services/modelService'; import { ICommandService } from 'vs/platform/commands/common/commands'; import { SAVE_FILE_COMMAND_ID, REVERT_FILE_COMMAND_ID, SAVE_FILE_AS_COMMAND_ID, SAVE_FILE_AS_LABEL } from 'vs/workbench/parts/files/electron-browser/fileCommands'; +import { createTextBufferFactoryFromSnapshot } from 'vs/editor/common/model/textModel'; export const CONFLICT_RESOLUTION_CONTEXT = 'saveConflictResolutionContext'; export const CONFLICT_RESOLUTION_SCHEME = 'conflictResolution'; @@ -262,7 +263,7 @@ export const acceptLocalChangesCommand = (accessor: ServicesAccessor, resource: resolverService.createModelReference(resource).then(reference => { const model = reference.object as ITextFileEditorModel; - const localModelValue = model.getValue(); + const localModelSnapshot = model.createSnapshot(); clearPendingResolveSaveConflictMessages(); // hide any previously shown message about how to use these actions @@ -270,7 +271,7 @@ export const acceptLocalChangesCommand = (accessor: ServicesAccessor, resource: return model.revert().then(() => { // Restore user value (without loosing undo stack) - modelService.updateModel(model.textEditorModel, localModelValue); + modelService.updateModel(model.textEditorModel, createTextBufferFactoryFromSnapshot(localModelSnapshot)); // Trigger save return model.save().then(() => { diff --git a/src/vs/workbench/services/textfile/common/textFileService.ts b/src/vs/workbench/services/textfile/common/textFileService.ts index a1ef38dd364e6..42f1d46637001 100644 --- a/src/vs/workbench/services/textfile/common/textFileService.ts +++ b/src/vs/workbench/services/textfile/common/textFileService.ts @@ -32,6 +32,8 @@ import { Schemas } from 'vs/base/common/network'; import { IHistoryService } from 'vs/workbench/services/history/common/history'; import { IRevertOptions } from 'vs/platform/editor/common/editor'; import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; +import { createTextBufferFactoryFromSnapshot } from 'vs/editor/common/model/textModel'; +import { IModelService } from 'vs/editor/common/services/modelService'; export interface IBackupResult { didBackup: boolean; @@ -73,7 +75,8 @@ export abstract class TextFileService implements ITextFileService { private backupFileService: IBackupFileService, private windowsService: IWindowsService, private historyService: IHistoryService, - contextKeyService: IContextKeyService + contextKeyService: IContextKeyService, + private modelService: IModelService ) { this.toUnbind = []; @@ -615,7 +618,7 @@ export abstract class TextFileService implements ITextFileService { // take over encoding and model value from source model targetModel.updatePreferredEncoding(sourceModel.getEncoding()); - targetModel.textEditorModel.setValue(sourceModel.getValue()); + this.modelService.updateModel(targetModel.textEditorModel, createTextBufferFactoryFromSnapshot(sourceModel.createSnapshot())); // save model return targetModel.save(options); diff --git a/src/vs/workbench/services/textfile/electron-browser/textFileService.ts b/src/vs/workbench/services/textfile/electron-browser/textFileService.ts index ecf4229c83b9a..1dcfc4c6334c0 100644 --- a/src/vs/workbench/services/textfile/electron-browser/textFileService.ts +++ b/src/vs/workbench/services/textfile/electron-browser/textFileService.ts @@ -30,6 +30,7 @@ import { IWindowsService, IWindowService } from 'vs/platform/windows/common/wind import { IHistoryService } from 'vs/workbench/services/history/common/history'; import { mnemonicButtonLabel } from 'vs/base/common/labels'; import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; +import { IModelService } from 'vs/editor/common/services/modelService'; export class TextFileService extends AbstractTextFileService { @@ -41,6 +42,7 @@ export class TextFileService extends AbstractTextFileService { @IInstantiationService instantiationService: IInstantiationService, @IConfigurationService configurationService: IConfigurationService, @IModeService private modeService: IModeService, + @IModelService modelService: IModelService, @IWindowService private windowService: IWindowService, @IEnvironmentService environmentService: IEnvironmentService, @IMessageService messageService: IMessageService, @@ -49,7 +51,7 @@ export class TextFileService extends AbstractTextFileService { @IHistoryService historyService: IHistoryService, @IContextKeyService contextKeyService: IContextKeyService ) { - super(lifecycleService, contextService, configurationService, fileService, untitledEditorService, instantiationService, messageService, environmentService, backupFileService, windowsService, historyService, contextKeyService); + super(lifecycleService, contextService, configurationService, fileService, untitledEditorService, instantiationService, messageService, environmentService, backupFileService, windowsService, historyService, contextKeyService, modelService); } public resolveTextContent(resource: URI, options?: IResolveContentOptions): TPromise { diff --git a/src/vs/workbench/test/workbenchTestServices.ts b/src/vs/workbench/test/workbenchTestServices.ts index 50f9c781ff861..e5db572cc9115 100644 --- a/src/vs/workbench/test/workbenchTestServices.ts +++ b/src/vs/workbench/test/workbenchTestServices.ts @@ -180,9 +180,10 @@ export class TestTextFileService extends TextFileService { @IBackupFileService backupFileService: IBackupFileService, @IWindowsService windowsService: IWindowsService, @IHistoryService historyService: IHistoryService, - @IContextKeyService contextKeyService: IContextKeyService + @IContextKeyService contextKeyService: IContextKeyService, + @IModelService modelService: IModelService ) { - super(lifecycleService, contextService, configurationService, fileService, untitledEditorService, instantiationService, messageService, TestEnvironmentService, backupFileService, windowsService, historyService, contextKeyService); + super(lifecycleService, contextService, configurationService, fileService, untitledEditorService, instantiationService, messageService, TestEnvironmentService, backupFileService, windowsService, historyService, contextKeyService, modelService); } public setPromptPath(path: string): void { From c6d150ebb49217241b7ba629c2fbc7f0c446fd97 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 22 Jan 2018 18:06:35 +0100 Subject: [PATCH 9/9] remove textEditorModel.getValue --- .../common/editor/textEditorModel.ts | 14 +------------ .../test/browser/fileEditorTracker.test.ts | 6 +++--- .../parts/search/browser/replaceService.ts | 3 ++- .../editor/test/browser/editorService.test.ts | 3 ++- .../services/textfile/common/textfiles.ts | 2 -- .../textfile/test/textFileEditorModel.test.ts | 4 ++-- .../test/textModelResolverService.test.ts | 3 ++- .../common/editor/resourceEditorInput.test.ts | 3 ++- .../test/common/editor/untitledEditor.test.ts | 3 ++- .../api/mainThreadSaveParticipant.test.ts | 21 ++++++++++--------- 10 files changed, 27 insertions(+), 35 deletions(-) diff --git a/src/vs/workbench/common/editor/textEditorModel.ts b/src/vs/workbench/common/editor/textEditorModel.ts index 8df2f87b499e0..8c10065dfcc6a 100644 --- a/src/vs/workbench/common/editor/textEditorModel.ts +++ b/src/vs/workbench/common/editor/textEditorModel.ts @@ -5,7 +5,7 @@ 'use strict'; import { TPromise } from 'vs/base/common/winjs.base'; -import { EndOfLinePreference, ITextModel, ITextBufferFactory } from 'vs/editor/common/model'; +import { ITextModel, ITextBufferFactory } from 'vs/editor/common/model'; import { IMode } from 'vs/editor/common/modes'; import { EditorModel } from 'vs/workbench/common/editor'; import URI from 'vs/base/common/uri'; @@ -141,18 +141,6 @@ export abstract class BaseTextEditorModel extends EditorModel implements ITextEd this.modelService.updateModel(this.textEditorModel, newValue); } - /** - * Returns the textual value of this editor model or null if it has not yet been created. - */ - public getValue(): string { - const model = this.textEditorModel; - if (model) { - return model.getValue(EndOfLinePreference.TextDefined, true /* Preserve BOM */); - } - - return null; - } - public createSnapshot(): ITextSnapshot { const model = this.textEditorModel; if (model) { diff --git a/src/vs/workbench/parts/files/test/browser/fileEditorTracker.test.ts b/src/vs/workbench/parts/files/test/browser/fileEditorTracker.test.ts index 4b9590f037f44..a76038659ec0b 100644 --- a/src/vs/workbench/parts/files/test/browser/fileEditorTracker.test.ts +++ b/src/vs/workbench/parts/files/test/browser/fileEditorTracker.test.ts @@ -16,7 +16,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { IEditorGroupService } from 'vs/workbench/services/group/common/groupService'; import { EditorStacksModel } from 'vs/workbench/common/editor/editorStacksModel'; import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles'; -import { FileOperation, FileOperationEvent, FileChangesEvent, FileChangeType, IFileService } from 'vs/platform/files/common/files'; +import { FileOperation, FileOperationEvent, FileChangesEvent, FileChangeType, IFileService, snapshotToString } from 'vs/platform/files/common/files'; import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel'; import { once } from 'vs/base/common/event'; @@ -191,14 +191,14 @@ suite('Files - FileEditorTracker', () => { accessor.textFileService.models.loadOrCreate(resource).then((model: TextFileEditorModel) => { model.textEditorModel.setValue('Super Good'); - assert.equal(model.getValue(), 'Super Good'); + assert.equal(snapshotToString(model.createSnapshot()), 'Super Good'); model.save().then(() => { // change event (watcher) accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.UPDATED }])); - assert.equal(model.getValue(), 'Hello Html'); + assert.equal(snapshotToString(model.createSnapshot()), 'Hello Html'); tracker.dispose(); diff --git a/src/vs/workbench/parts/search/browser/replaceService.ts b/src/vs/workbench/parts/search/browser/replaceService.ts index c93549e85dd49..fca4c6b1a3678 100644 --- a/src/vs/workbench/parts/search/browser/replaceService.ts +++ b/src/vs/workbench/parts/search/browser/replaceService.ts @@ -24,6 +24,7 @@ import { ScrollType } from 'vs/editor/common/editorCommon'; import { ITextModel } from 'vs/editor/common/model'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IFileService } from 'vs/platform/files/common/files'; +import { createTextBufferFactoryFromSnapshot } from 'vs/editor/common/model/textModel'; const REPLACE_PREVIEW = 'replacePreview'; @@ -70,7 +71,7 @@ class ReplacePreviewModel extends Disposable { ref = this._register(ref); const sourceModel = ref.object.textEditorModel; const sourceModelModeId = sourceModel.getLanguageIdentifier().language; - const replacePreviewModel = this.modelService.createModel(sourceModel.getValue(), this.modeService.getOrCreateMode(sourceModelModeId), replacePreviewUri); + const replacePreviewModel = this.modelService.createModel(createTextBufferFactoryFromSnapshot(sourceModel.createSnapshot()), this.modeService.getOrCreateMode(sourceModelModeId), replacePreviewUri); this._register(fileMatch.onChange(modelChange => this.update(sourceModel, replacePreviewModel, fileMatch, modelChange))); this._register(this.searchWorkbenchService.searchModel.onReplaceTermChanged(() => this.update(sourceModel, replacePreviewModel, fileMatch))); this._register(fileMatch.onDispose(() => replacePreviewModel.dispose())); // TODO@Sandeep we should not dispose a model directly but rather the reference (depends on https://github.com/Microsoft/vscode/issues/17073) diff --git a/src/vs/workbench/services/editor/test/browser/editorService.test.ts b/src/vs/workbench/services/editor/test/browser/editorService.test.ts index 196825b2f11ac..e5fd893ab7363 100644 --- a/src/vs/workbench/services/editor/test/browser/editorService.test.ts +++ b/src/vs/workbench/services/editor/test/browser/editorService.test.ts @@ -19,6 +19,7 @@ import { UntitledEditorInput } from 'vs/workbench/common/editor/untitledEditorIn import { ResourceEditorInput } from 'vs/workbench/common/editor/resourceEditorInput'; import { TestThemeService } from 'vs/platform/theme/test/common/testThemeService'; import { ICloseEditorsFilter } from 'vs/workbench/browser/parts/editor/editorPart'; +import { snapshotToString } from 'vs/platform/files/common/files'; let activeEditor: BaseEditor = { getSelection: function () { @@ -163,7 +164,7 @@ suite('WorkbenchEditorService', () => { const untitledInput = openedEditorInput as UntitledEditorInput; untitledInput.resolve().then(model => { - assert.equal(model.getValue(), 'Hello Untitled'); + assert.equal(snapshotToString(model.createSnapshot()), 'Hello Untitled'); }); }); diff --git a/src/vs/workbench/services/textfile/common/textfiles.ts b/src/vs/workbench/services/textfile/common/textfiles.ts index 6000c9734e52e..eacc46d19705f 100644 --- a/src/vs/workbench/services/textfile/common/textfiles.ts +++ b/src/vs/workbench/services/textfile/common/textfiles.ts @@ -200,8 +200,6 @@ export interface ITextFileEditorModel extends ITextEditorModel, IEncodingSupport revert(soft?: boolean): TPromise; - getValue(): string; - createSnapshot(): ITextSnapshot; isDirty(): boolean; diff --git a/src/vs/workbench/services/textfile/test/textFileEditorModel.test.ts b/src/vs/workbench/services/textfile/test/textFileEditorModel.test.ts index 5e36065442d70..d747bb23b3f35 100644 --- a/src/vs/workbench/services/textfile/test/textFileEditorModel.test.ts +++ b/src/vs/workbench/services/textfile/test/textFileEditorModel.test.ts @@ -14,7 +14,7 @@ import { ITextFileService, ModelState, StateChange } from 'vs/workbench/services import { workbenchInstantiationService, TestTextFileService, createFileInput, TestFileService } from 'vs/workbench/test/workbenchTestServices'; import { onError, toResource } from 'vs/base/test/common/utils'; import { TextFileEditorModelManager } from 'vs/workbench/services/textfile/common/textFileEditorModelManager'; -import { FileOperationResult, FileOperationError, IFileService } from 'vs/platform/files/common/files'; +import { FileOperationResult, FileOperationError, IFileService, snapshotToString } from 'vs/platform/files/common/files'; import { IModelService } from 'vs/editor/common/services/modelService'; class ServiceAccessor { @@ -284,7 +284,7 @@ suite('Files - TextFileEditorModel', () => { model.onDidStateChange(e => { if (e === StateChange.SAVED) { - assert.equal(model.getValue(), 'bar'); + assert.equal(snapshotToString(model.createSnapshot()), 'bar'); assert.ok(!model.isDirty()); eventCounter++; } diff --git a/src/vs/workbench/services/textmodelResolver/test/textModelResolverService.test.ts b/src/vs/workbench/services/textmodelResolver/test/textModelResolverService.test.ts index f5bde377342c7..3c904f154a425 100644 --- a/src/vs/workbench/services/textmodelResolver/test/textModelResolverService.test.ts +++ b/src/vs/workbench/services/textmodelResolver/test/textModelResolverService.test.ts @@ -22,6 +22,7 @@ import { ITextFileService } from 'vs/workbench/services/textfile/common/textfile import { IUntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService'; import { TextFileEditorModelManager } from 'vs/workbench/services/textfile/common/textFileEditorModelManager'; import { once } from 'vs/base/common/event'; +import { snapshotToString } from 'vs/platform/files/common/files'; class ServiceAccessor { constructor( @@ -73,7 +74,7 @@ suite('Workbench - TextModelResolverService', () => { input.resolve().then(model => { assert.ok(model); - assert.equal((model as ResourceEditorModel).getValue(), 'Hello Test'); + assert.equal(snapshotToString((model as ResourceEditorModel).createSnapshot()), 'Hello Test'); let disposed = false; once(model.onDispose)(() => { diff --git a/src/vs/workbench/test/common/editor/resourceEditorInput.test.ts b/src/vs/workbench/test/common/editor/resourceEditorInput.test.ts index 85c63eb3e1bbb..bed57cb0b054a 100644 --- a/src/vs/workbench/test/common/editor/resourceEditorInput.test.ts +++ b/src/vs/workbench/test/common/editor/resourceEditorInput.test.ts @@ -13,6 +13,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { workbenchInstantiationService } from 'vs/workbench/test/workbenchTestServices'; import { IModelService } from 'vs/editor/common/services/modelService'; import { IModeService } from 'vs/editor/common/services/modeService'; +import { snapshotToString } from 'vs/platform/files/common/files'; class ServiceAccessor { constructor( @@ -39,7 +40,7 @@ suite('Workbench - ResourceEditorInput', () => { return input.resolve().then((model: ResourceEditorModel) => { assert.ok(model); - assert.equal(model.getValue(), 'function test() {}'); + assert.equal(snapshotToString(model.createSnapshot()), 'function test() {}'); }); }); }); \ No newline at end of file diff --git a/src/vs/workbench/test/common/editor/untitledEditor.test.ts b/src/vs/workbench/test/common/editor/untitledEditor.test.ts index b7b8a04dd4a0a..8edf3bb24aae8 100644 --- a/src/vs/workbench/test/common/editor/untitledEditor.test.ts +++ b/src/vs/workbench/test/common/editor/untitledEditor.test.ts @@ -17,6 +17,7 @@ import { UntitledEditorModel } from 'vs/workbench/common/editor/untitledEditorMo import { IModeService } from 'vs/editor/common/services/modeService'; import { ModeServiceImpl } from 'vs/editor/common/services/modeServiceImpl'; import { UntitledEditorInput } from 'vs/workbench/common/editor/untitledEditorInput'; +import { snapshotToString } from 'vs/platform/files/common/files'; export class TestUntitledEditorService extends UntitledEditorService { @@ -142,7 +143,7 @@ suite('Workbench - Untitled Editor', () => { assert.ok(!model1.isDirty()); return service.loadOrCreate({ initialValue: 'Hello World' }).then(model2 => { - assert.equal(model2.getValue(), 'Hello World'); + assert.equal(snapshotToString(model2.createSnapshot()), 'Hello World'); const input = service.createOrGet(); diff --git a/src/vs/workbench/test/electron-browser/api/mainThreadSaveParticipant.test.ts b/src/vs/workbench/test/electron-browser/api/mainThreadSaveParticipant.test.ts index d6f3681d33193..d83503b679ec9 100644 --- a/src/vs/workbench/test/electron-browser/api/mainThreadSaveParticipant.test.ts +++ b/src/vs/workbench/test/electron-browser/api/mainThreadSaveParticipant.test.ts @@ -17,6 +17,7 @@ import { Selection } from 'vs/editor/common/core/selection'; import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel'; import { ITextFileService, SaveReason } from 'vs/workbench/services/textfile/common/textfiles'; import { TextFileEditorModelManager } from 'vs/workbench/services/textfile/common/textFileEditorModelManager'; +import { snapshotToString } from 'vs/platform/files/common/files'; class ServiceAccessor { constructor( @ITextFileService public textFileService: TestTextFileService, @IModelService public modelService: IModelService) { @@ -51,25 +52,25 @@ suite('MainThreadSaveParticipant', function () { let lineContent = ''; model.textEditorModel.setValue(lineContent); participant.participate(model, { reason: SaveReason.EXPLICIT }); - assert.equal(model.getValue(), lineContent); + assert.equal(snapshotToString(model.createSnapshot()), lineContent); // No new line if last line already empty lineContent = `Hello New Line${model.textEditorModel.getEOL()}`; model.textEditorModel.setValue(lineContent); participant.participate(model, { reason: SaveReason.EXPLICIT }); - assert.equal(model.getValue(), lineContent); + assert.equal(snapshotToString(model.createSnapshot()), lineContent); // New empty line added (single line) lineContent = 'Hello New Line'; model.textEditorModel.setValue(lineContent); participant.participate(model, { reason: SaveReason.EXPLICIT }); - assert.equal(model.getValue(), `${lineContent}${model.textEditorModel.getEOL()}`); + assert.equal(snapshotToString(model.createSnapshot()), `${lineContent}${model.textEditorModel.getEOL()}`); // New empty line added (multi line) lineContent = `Hello New Line${model.textEditorModel.getEOL()}Hello New Line${model.textEditorModel.getEOL()}Hello New Line`; model.textEditorModel.setValue(lineContent); participant.participate(model, { reason: SaveReason.EXPLICIT }); - assert.equal(model.getValue(), `${lineContent}${model.textEditorModel.getEOL()}`); + assert.equal(snapshotToString(model.createSnapshot()), `${lineContent}${model.textEditorModel.getEOL()}`); done(); }); @@ -91,25 +92,25 @@ suite('MainThreadSaveParticipant', function () { let lineContent = `${textContent}`; model.textEditorModel.setValue(lineContent); participant.participate(model, { reason: SaveReason.EXPLICIT }); - assert.equal(model.getValue(), lineContent); + assert.equal(snapshotToString(model.createSnapshot()), lineContent); // No new line removal if last line is single new line lineContent = `${textContent}${eol}`; model.textEditorModel.setValue(lineContent); participant.participate(model, { reason: SaveReason.EXPLICIT }); - assert.equal(model.getValue(), lineContent); + assert.equal(snapshotToString(model.createSnapshot()), lineContent); // Remove new line (single line with two new lines) lineContent = `${textContent}${eol}${eol}`; model.textEditorModel.setValue(lineContent); participant.participate(model, { reason: SaveReason.EXPLICIT }); - assert.equal(model.getValue(), `${textContent}${eol}`); + assert.equal(snapshotToString(model.createSnapshot()), `${textContent}${eol}`); // Remove new lines (multiple lines with multiple new lines) lineContent = `${textContent}${eol}${textContent}${eol}${eol}${eol}`; model.textEditorModel.setValue(lineContent); participant.participate(model, { reason: SaveReason.EXPLICIT }); - assert.equal(model.getValue(), `${textContent}${eol}${textContent}${eol}`); + assert.equal(snapshotToString(model.createSnapshot()), `${textContent}${eol}${textContent}${eol}`); done(); }); @@ -134,11 +135,11 @@ suite('MainThreadSaveParticipant', function () { model.textEditorModel.pushEditOperations([new Selection(1, 14, 1, 14)], textEdits, () => { return [new Selection(1, 15, 1, 15)]; }); // undo model.textEditorModel.undo(); - assert.equal(model.getValue(), `${textContent}`); + assert.equal(snapshotToString(model.createSnapshot()), `${textContent}`); // trim final new lines should not mess the undo stack participant.participate(model, { reason: SaveReason.EXPLICIT }); model.textEditorModel.redo(); - assert.equal(model.getValue(), `${textContent}.`); + assert.equal(snapshotToString(model.createSnapshot()), `${textContent}.`); done(); }); });