Skip to content

Commit

Permalink
Use r+ with truncation when saving existing files (#31733)
Browse files Browse the repository at this point in the history
* Use r+ with truncation when saving existing files

'w' doesn't work with hidden files.

Fixes: #931

* use r+ only if EPERM is caught

* add check for Windows

* move writing to a separate method

* nits

* remove string overload for options

* remove encoding property from writeFileAndFlush
  • Loading branch information
seishun authored and bpasero committed Aug 15, 2017
1 parent a8d63d3 commit cc9d8fc
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 26 deletions.
8 changes: 3 additions & 5 deletions src/vs/base/node/extfs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,15 +332,13 @@ export function mv(source: string, target: string, callback: (error: Error) => v
//
// 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: { encoding?: string; mode?: number; flag?: string; }, callback: (error: Error) => void): void {
export function writeFileAndFlush(path: string, data: string | NodeBuffer, options: { mode?: number; flag?: string; }, callback: (error: Error) => void): void {
if (!canFlush) {
return fs.writeFile(path, data, options, callback);
}

if (!options) {
options = { encoding: 'utf8', mode: 0o666, flag: 'w' };
} else if (typeof options === 'string') {
options = { encoding: <string>options, mode: 0o666, flag: 'w' };
options = { mode: 0o666, flag: 'w' };
}

// Open the file with same flags and mode as fs.writeFile()
Expand All @@ -350,7 +348,7 @@ export function writeFileAndFlush(path: string, data: string | NodeBuffer, optio
}

// It is valid to pass a fd handle to fs.writeFile() and this will keep the handle open!
fs.writeFile(fd, data, options.encoding, (writeError) => {
fs.writeFile(fd, data, (writeError) => {
if (writeError) {
return fs.close(fd, () => callback(writeError)); // still need to close the handle on error!
}
Expand Down
12 changes: 8 additions & 4 deletions src/vs/base/node/pfs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ export function touch(path: string): TPromise<void> {
return nfcall(fs.utimes, path, now, now);
}

export function truncate(path: string, len: number): TPromise<void> {
return nfcall(fs.truncate, path, len);
}

export function readFile(path: string): TPromise<Buffer>;
export function readFile(path: string, encoding: string): TPromise<string>;
export function readFile(path: string, encoding?: string): TPromise<Buffer | string> {
Expand All @@ -120,12 +124,12 @@ export function readFile(path: string, encoding?: string): TPromise<Buffer | str
// Therefor we use a Queue on the path that is given to us to sequentialize calls to the same path properly.
const writeFilePathQueue: { [path: string]: Queue<void> } = Object.create(null);

export function writeFile(path: string, data: string, encoding?: string): TPromise<void>;
export function writeFile(path: string, data: NodeBuffer, encoding?: string): TPromise<void>;
export function writeFile(path: string, data: any, encoding: string = 'utf8'): TPromise<void> {
export function writeFile(path: string, data: string, options?: { mode?: number; flag?: string; }): TPromise<void>;
export function writeFile(path: string, data: NodeBuffer, options?: { mode?: number; flag?: string; }): TPromise<void>;
export function writeFile(path: string, data: any, options?: { mode?: number; flag?: string; }): TPromise<void> {
let queueKey = toQueueKey(path);

return ensureWriteFileQueue(queueKey).queue(() => nfcall(extfs.writeFileAndFlush, path, data, encoding));
return ensureWriteFileQueue(queueKey).queue(() => nfcall(extfs.writeFileAndFlush, path, data, options));
}

function toQueueKey(path: string): string {
Expand Down
51 changes: 34 additions & 17 deletions src/vs/workbench/services/files/node/fileService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,30 +348,47 @@ export class FileService implements IFileService {

// 3.) check to add UTF BOM
return addBomPromise.then(addBom => {
let writeFilePromise: TPromise<void>;

// Write fast if we do UTF 8 without BOM
if (!addBom && encodingToWrite === encoding.UTF8) {
writeFilePromise = pfs.writeFile(absolutePath, value, encoding.UTF8);
}

// Otherwise use encoding lib
else {
const encoded = encoding.encode(value, encodingToWrite, { addBOM: addBom });
writeFilePromise = pfs.writeFile(absolutePath, encoded);
}

// 4.) set contents
return writeFilePromise.then(() => {
// 4.) set contents and resolve
return this.doSetContentsAndResolve(resource, absolutePath, value, addBom, encodingToWrite, { mode: 0o666, flag: 'w' }).then(undefined, error => {
// Can't use 'w' for hidden files on Windows (see https://github.com/Microsoft/vscode/issues/931)
if (!exists || error.code !== 'EPERM' || !isWindows) {
return TPromise.wrapError(error);
}

// 5.) resolve
return this.resolve(resource);
// 'r+' always works for existing files, but we need to truncate manually
// 5.) truncate
return pfs.truncate(absolutePath, 0).then(() => {
// 6.) set contents and resolve again
return this.doSetContentsAndResolve(resource, absolutePath, value, addBom, encodingToWrite, { mode: 0o666, flag: 'r+' });
});
});
});
});
});
}

private doSetContentsAndResolve(resource: uri, absolutePath: string, value: string, addBOM: boolean, encodingToWrite: string, options: { mode?: number; flag?: string; }): TPromise<IFileStat> {
let writeFilePromise: TPromise<void>;

// Write fast if we do UTF 8 without BOM
if (!addBOM && encodingToWrite === encoding.UTF8) {
writeFilePromise = pfs.writeFile(absolutePath, value, options);
}

// Otherwise use encoding lib
else {
const encoded = encoding.encode(value, encodingToWrite, { addBOM });
writeFilePromise = pfs.writeFile(absolutePath, encoded, options);
}

// set contents
return writeFilePromise.then(() => {

// resolve
return this.resolve(resource);
});
}

public createFile(resource: uri, content: string = ''): TPromise<IFileStat> {

// Create file
Expand Down

0 comments on commit cc9d8fc

Please sign in to comment.