Skip to content

Commit

Permalink
Revert "debt - adopt new fs.readdir with stat info"
Browse files Browse the repository at this point in the history
This reverts commit da5fb7d.
  • Loading branch information
bpasero committed Aug 12, 2019
1 parent 07e9310 commit 956208b
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 60 deletions.
14 changes: 0 additions & 14 deletions src/vs/base/node/pfs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,20 +138,6 @@ export async function readdir(path: string): Promise<string[]> {
return handleDirectoryChildren(await promisify(fs.readdir)(path));
}

export async function readdirWithFileTypes(path: string): Promise<fs.Dirent[]> {
const children = await promisify(fs.readdir)(path, { withFileTypes: true });

// Mac: uses NFD unicode form on disk, but we want NFC
// See also https://github.com/nodejs/node/issues/2165
if (platform.isMacintosh) {
for (const child of children) {
child.name = normalizeNFC(child.name);
}
}

return children;
}

export function readdirSync(path: string): string[] {
return handleDirectoryChildren(fs.readdirSync(path));
}
Expand Down
26 changes: 0 additions & 26 deletions src/vs/base/test/node/pfs/pfs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { CancellationTokenSource } from 'vs/base/common/cancellation';
import { isWindows, isLinux } from 'vs/base/common/platform';
import { canNormalize } from 'vs/base/common/normalization';
import { VSBuffer } from 'vs/base/common/buffer';
import { join } from 'path';

const chunkSize = 64 * 1024;
const readError = 'Error while reading';
Expand Down Expand Up @@ -387,31 +386,6 @@ suite('PFS', () => {
}
});

test('readdirWithFileTypes', async () => {
if (canNormalize && typeof process.versions['electron'] !== 'undefined' /* needs electron */) {
const id = uuid.generateUuid();
const parentDir = path.join(os.tmpdir(), 'vsctests', id);
const testDir = join(parentDir, 'pfs', id);

const newDir = path.join(testDir, 'öäü');
await pfs.mkdirp(newDir, 493);

await pfs.writeFile(join(testDir, 'somefile.txt'), 'contents');

assert.ok(fs.existsSync(newDir));

const children = await pfs.readdirWithFileTypes(testDir);

assert.equal(children.some(n => n.name === 'öäü'), true); // Mac always converts to NFD, so
assert.equal(children.some(n => n.isDirectory()), true);

assert.equal(children.some(n => n.name === 'somefile.txt'), true);
assert.equal(children.some(n => n.isFile()), true);

await pfs.rimraf(parentDir);
}
});

test('writeFile (string)', async () => {
const smallData = 'Hello World';
const bigData = (new Array(100 * 1024)).join('Large String\n');
Expand Down
33 changes: 13 additions & 20 deletions src/vs/platform/files/node/diskFileSystemProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { mkdir, open, close, read, write, fdatasync, Dirent, Stats } from 'fs';
import { mkdir, open, close, read, write, fdatasync } from 'fs';
import { promisify } from 'util';
import { IDisposable, Disposable, toDisposable, dispose, combinedDisposable } from 'vs/base/common/lifecycle';
import { IFileSystemProvider, FileSystemProviderCapabilities, IFileChange, IWatchOptions, IStat, FileType, FileDeleteOptions, FileOverwriteOptions, FileWriteOptions, FileOpenOptions, FileSystemProviderErrorCode, createFileSystemProviderError, FileSystemProviderError } from 'vs/platform/files/common/files';
import { URI } from 'vs/base/common/uri';
import { Event, Emitter } from 'vs/base/common/event';
import { isLinux, isWindows } from 'vs/base/common/platform';
import { statLink, unlink, move, copy, readFile, truncate, rimraf, RimRafMode, exists, readdirWithFileTypes } from 'vs/base/node/pfs';
import { statLink, readdir, unlink, move, copy, readFile, truncate, rimraf, RimRafMode, exists } from 'vs/base/node/pfs';
import { normalize, basename, dirname } from 'vs/base/common/path';
import { joinPath } from 'vs/base/common/resources';
import { isEqual } from 'vs/base/common/extpath';
Expand Down Expand Up @@ -62,8 +62,15 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
try {
const { stat, isSymbolicLink } = await statLink(this.toFilePath(resource)); // cannot use fs.stat() here to support links properly

let type: number;
if (isSymbolicLink) {
type = FileType.SymbolicLink | (stat.isDirectory() ? FileType.Directory : FileType.File);
} else {
type = stat.isFile() ? FileType.File : stat.isDirectory() ? FileType.Directory : FileType.Unknown;
}

return {
type: this.toType(stat, isSymbolicLink),
type,
ctime: stat.ctime.getTime(),
mtime: stat.mtime.getTime(),
size: stat.size
Expand All @@ -75,19 +82,13 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro

async readdir(resource: URI): Promise<[string, FileType][]> {
try {
const children = await readdirWithFileTypes(this.toFilePath(resource));
const children = await readdir(this.toFilePath(resource));

const result: [string, FileType][] = [];
await Promise.all(children.map(async child => {
try {
let type: FileType;
if (child.isSymbolicLink()) {
type = (await this.stat(joinPath(resource, child.name))).type; // always resolve target the link points to if any
} else {
type = this.toType(child);
}

result.push([child.name, type]);
const stat = await this.stat(joinPath(resource, child));
result.push([child, stat.type]);
} catch (error) {
this.logService.trace(error); // ignore errors for individual entries that can arise from permission denied
}
Expand All @@ -99,14 +100,6 @@ export class DiskFileSystemProvider extends Disposable implements IFileSystemPro
}
}

private toType(entry: Stats | Dirent, isSymbolicLink = entry.isSymbolicLink()): FileType {
if (isSymbolicLink) {
return FileType.SymbolicLink | (entry.isDirectory() ? FileType.Directory : FileType.File);
}

return entry.isFile() ? FileType.File : entry.isDirectory() ? FileType.Directory : FileType.Unknown;
}

//#endregion

//#region File Reading/Writing
Expand Down

3 comments on commit 956208b

@eamodio
Copy link
Contributor

@eamodio eamodio commented on 956208b Sep 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bpasero I'm curious as to why this was reverted? I'm currently using readdir with withFileTypes in GitLens and wondering if there is some lurking issues.

@bpasero
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eamodio issues with symbolic links: nodejs/node#22808

@eamodio
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bpasero thank you!

Please sign in to comment.