Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

show symbolic links as decorations in explorer #43815

Merged
merged 6 commits into from
Feb 21, 2018
Merged

show symbolic links as decorations in explorer #43815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
40b7a64
show symbolic links as decorations in explorer
isidorn Feb 16, 2018
e3a88e9
Merge branch 'master' into isidorn/symbolicLinks
isidorn Feb 20, 2018
48daad5
pfs, extfs: add statLink
isidorn Feb 20, 2018
92e7f72
stat link unit tests
isidorn Feb 20, 2018
6ec78d6
use pfs.statLink
isidorn Feb 20, 2018
5e23401
polish
isidorn Feb 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion src/vs/base/node/extfs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,26 @@ export function readdir(path: string, callback: (error: Error, files: string[])
return fs.readdir(path, callback);
}

export function statLink(path: string, callback: (error: Error, statAndIsLink: { stat: fs.Stats, isSymbolicLink: boolean }) => void): void {
fs.lstat(path, (error, stat) => {
if (error) {
return callback(error, null);
}

if (stat.isSymbolicLink()) {
fs.stat(path, (error, stat) => {
if (error) {
return callback(error, null);
}

callback(null, { stat, isSymbolicLink: true });
});
} else {
callback(null, { stat, isSymbolicLink: false });
}
});
}

export function copy(source: string, target: string, callback: (error: Error) => void, copiedSources?: { [path: string]: boolean }): void {
if (!copiedSources) {
copiedSources = Object.create(null);
Expand Down Expand Up @@ -628,4 +648,4 @@ export function watch(path: string, onChange: (type: string, path: string) => vo
}

return void 0;
}
}
4 changes: 4 additions & 0 deletions src/vs/base/node/pfs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ export function stat(path: string): TPromise<fs.Stats> {
return nfcall(fs.stat, path);
}

export function statLink(path: string): TPromise<{ stat: fs.Stats, isSymbolicLink: boolean }> {
Copy link
Member

@bpasero bpasero Feb 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@isidorn why is this method not simply using extfs? E.g. it should be as simple as:

export function statLink(path: string): TPromise<string> {
	return nfcall(extfs.statLink, path);
}

Copy link
Contributor Author

@isidorn isidorn Feb 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@bpasero you are correct. I was not aware I can depend on extfs in pfs, but now I see it is already done.
Fixed

return nfcall(extfs.statLink, path);
}

export function lstat(path: string): TPromise<fs.Stats> {
return nfcall(fs.lstat, path);
}
Expand Down
37 changes: 36 additions & 1 deletion src/vs/base/test/node/extfs/extfs.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,41 @@ suite('Extfs', () => {
}); // 493 = 0755
});

test('stat link', function (done: () => void) {
const id1 = uuid.generateUuid();
const parentDir = path.join(os.tmpdir(), 'vsctests', id1);
const directory = path.join(parentDir, 'extfs', id1);

const id2 = uuid.generateUuid();
const symbolicLink = path.join(parentDir, 'extfs', id2);

mkdirp(directory, 493, error => {
if (error) {
return onError(error, done);
}

fs.symlinkSync(directory, symbolicLink);

extfs.statLink(directory, (error, statAndIsLink) => {
if (error) {
return onError(error, done);
}

assert.ok(!statAndIsLink.isSymbolicLink);

extfs.statLink(symbolicLink, (error, statAndIsLink) => {
if (error) {
return onError(error, done);
}

assert.ok(statAndIsLink.isSymbolicLink);
extfs.delSync(directory);
done();
});
});
});
});

test('delSync - swallows file not found error', function () {
const id = uuid.generateUuid();
const parentDir = path.join(os.tmpdir(), 'vsctests', id);
Expand Down Expand Up @@ -524,4 +559,4 @@ suite('Extfs', () => {
extfs.del(parentDir, os.tmpdir(), done, ignore);
});
});
});
});
5 changes: 5 additions & 0 deletions src/vs/platform/files/common/files.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,11 @@ export interface IFileStat extends IBaseStat {
*/
isDirectory: boolean;

/**
* The resource is a symbolic link.
*/
isSymbolicLink?: boolean;

/**
* The children of the file stat or undefined if none.
*/
Expand Down
12 changes: 9 additions & 3 deletions src/vs/workbench/parts/files/common/explorerModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,17 @@ export class FileStat implements IFileStat {
public mtime: number;
public etag: string;
private _isDirectory: boolean;
private _isSymbolicLink: boolean;
public children: FileStat[];
public parent: FileStat;

public isDirectoryResolved: boolean;

constructor(resource: URI, public root: FileStat, isDirectory?: boolean, name: string = getPathLabel(resource), mtime?: number, etag?: string) {
constructor(resource: URI, public root: FileStat, isSymbolicLink?: boolean, isDirectory?: boolean, name: string = getPathLabel(resource), mtime?: number, etag?: string) {
this.resource = resource;
this.name = name;
this.isDirectory = !!isDirectory;
this._isSymbolicLink = !!isSymbolicLink;
this.etag = etag;
this.mtime = mtime;

Expand All @@ -94,6 +96,10 @@ export class FileStat implements IFileStat {
this.isDirectoryResolved = false;
}

public get isSymbolicLink(): boolean {
return this._isSymbolicLink;
}

public get isDirectory(): boolean {
return this._isDirectory;
}
Expand Down Expand Up @@ -123,7 +129,7 @@ export class FileStat implements IFileStat {
}

public static create(raw: IFileStat, root: FileStat, resolveTo?: URI[]): FileStat {
const stat = new FileStat(raw.resource, root, raw.isDirectory, raw.name, raw.mtime, raw.etag);
const stat = new FileStat(raw.resource, root, raw.isSymbolicLink, raw.isDirectory, raw.name, raw.mtime, raw.etag);

// Recursively add children if present
if (stat.isDirectory) {
Expand Down Expand Up @@ -316,7 +322,7 @@ export class NewStatPlaceholder extends FileStat {
private directoryPlaceholder: boolean;

constructor(isDirectory: boolean, root: FileStat) {
super(URI.file(''), root, false, '');
super(URI.file(''), root, false, false, '');

this.id = NewStatPlaceholder.ID++;
this.isDirectoryResolved = isDirectory;
Expand Down
8 changes: 7 additions & 1 deletion src/vs/workbench/parts/files/electron-browser/views/explorerDecorationsProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,20 @@ export class ExplorerDecorationsProvider implements IDecorationsProvider {
}

provideDecorations(resource: URI): IDecorationData {
const fileStat = this.model.roots.filter(r => r.resource.toString() === resource.toString()).pop();
const fileStat = this.model.findClosest(resource);
if (fileStat && fileStat.nonexistentRoot) {
return {
tooltip: localize('canNotResolve', "Can not resolve workspace folder"),
letter: '!',
color: listInvalidItemForeground,
};
}
if (fileStat && fileStat.isSymbolicLink) {
return {
tooltip: localize('symbolicLlink', "Symbolic Link"),
letter: '\u2937'
};
}

return undefined;
}
Expand Down
12 changes: 6 additions & 6 deletions src/vs/workbench/parts/files/test/electron-browser/explorerModel.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { validateFileName } from 'vs/workbench/parts/files/electron-browser/file
import { FileStat } from 'vs/workbench/parts/files/common/explorerModel';

function createStat(path: string, name: string, isFolder: boolean, hasChildren: boolean, size: number, mtime: number): FileStat {
return new FileStat(toResource(path), undefined, isFolder, name, mtime);
return new FileStat(toResource(path), undefined, false, isFolder, name, mtime);
}

function toResource(path) {
Expand Down Expand Up @@ -239,20 +239,20 @@ suite('Files - View Model', () => {
test('Merge Local with Disk', function () {
const d = new Date().toUTCString();

const merge1 = new FileStat(URI.file(join('C:\\', '/path/to')), undefined, true, 'to', Date.now(), d);
const merge2 = new FileStat(URI.file(join('C:\\', '/path/to')), undefined, true, 'to', Date.now(), new Date(0).toUTCString());
const merge1 = new FileStat(URI.file(join('C:\\', '/path/to')), undefined, false, true, 'to', Date.now(), d);
const merge2 = new FileStat(URI.file(join('C:\\', '/path/to')), undefined, false, true, 'to', Date.now(), new Date(0).toUTCString());

// Merge Properties
FileStat.mergeLocalWithDisk(merge2, merge1);
assert.strictEqual(merge1.mtime, merge2.mtime);

// Merge Child when isDirectoryResolved=false is a no-op
merge2.addChild(new FileStat(URI.file(join('C:\\', '/path/to/foo.html')), undefined, true, 'foo.html', Date.now(), d));
merge2.addChild(new FileStat(URI.file(join('C:\\', '/path/to/foo.html')), undefined, false, true, 'foo.html', Date.now(), d));
FileStat.mergeLocalWithDisk(merge2, merge1);
assert.strictEqual(merge1.children.length, 0);

// Merge Child with isDirectoryResolved=true
const child = new FileStat(URI.file(join('C:\\', '/path/to/foo.html')), undefined, true, 'foo.html', Date.now(), d);
const child = new FileStat(URI.file(join('C:\\', '/path/to/foo.html')), undefined, false, true, 'foo.html', Date.now(), d);
merge2.removeChild(child);
merge2.addChild(child);
merge2.isDirectoryResolved = true;
Expand All @@ -266,4 +266,4 @@ suite('Files - View Model', () => {
FileStat.mergeLocalWithDisk(merge2, merge1);
assert.ok(existingChild === merge1.children[0]);
});
});
});
1 change: 1 addition & 0 deletions src/vs/workbench/services/files/electron-browser/remoteFileService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ function toIFileStat(provider: IFileSystemProvider, tuple: [URI, IStat], recurse
const [resource, stat] = tuple;
const fileStat: IFileStat = {
isDirectory: false,
isSymbolicLink: stat.type === FileType.Symlink,
resource: resource,
name: basename(resource.path),
mtime: stat.mtime,
Expand Down
34 changes: 18 additions & 16 deletions src/vs/workbench/services/files/node/fileService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -912,8 +912,8 @@ export class FileService implements IFileService {
private toStatResolver(resource: uri): TPromise<StatResolver> {
const absolutePath = this.toAbsolutePath(resource);

return pfs.stat(absolutePath).then(stat => {
return new StatResolver(resource, stat.isDirectory(), stat.mtime.getTime(), stat.size, this.options.verboseLogging ? this.options.errorLogger : void 0);
return pfs.statLink(absolutePath).then(({ isSymbolicLink, stat }) => {
return new StatResolver(resource, isSymbolicLink, stat.isDirectory(), stat.mtime.getTime(), stat.size, this.options.verboseLogging ? this.options.errorLogger : void 0);
});
}

Expand Down Expand Up @@ -1146,25 +1146,21 @@ export class FileService implements IFileService {
}

export class StatResolver {
private resource: uri;
private isDirectory: boolean;
private mtime: number;
private name: string;
private etag: string;
private size: number;
private errorLogger: (error: Error | string) => void;

constructor(resource: uri, isDirectory: boolean, mtime: number, size: number, errorLogger?: (error: Error | string) => void) {
constructor(
private resource: uri,
private isSymbolicLink: boolean,
private isDirectory: boolean,
private mtime: number,
private size: number,
private errorLogger?: (error: Error | string) => void
) {
assert.ok(resource && resource.scheme === Schemas.file, `Invalid resource: ${resource}`);

this.resource = resource;
this.isDirectory = isDirectory;
this.mtime = mtime;
this.name = getBaseLabel(resource);
this.etag = etag(size, mtime);
this.size = size;

this.errorLogger = errorLogger;
}

public resolve(options: IResolveFileOptions): TPromise<IFileStat> {
Expand All @@ -1173,6 +1169,7 @@ export class StatResolver {
const fileStat: IFileStat = {
resource: this.resource,
isDirectory: this.isDirectory,
isSymbolicLink: this.isSymbolicLink,
name: this.name,
etag: this.etag,
size: this.size,
Expand Down Expand Up @@ -1223,6 +1220,7 @@ export class StatResolver {
flow.parallel(files, (file: string, clb: (error: Error, children: IFileStat) => void) => {
const fileResource = uri.file(paths.resolve(absolutePath, file));
let fileStat: fs.Stats;
let isSymbolicLink = false;
const $this = this;

flow.sequence(
Expand All @@ -1235,7 +1233,10 @@ export class StatResolver {
},

function stat(this: any): void {
fs.stat(fileResource.fsPath, this);
extfs.statLink(fileResource.fsPath, (error: Error, statAndIsLink) => {
Copy link
Member

Choose a reason for hiding this comment

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

Can be nicer:

extfs.statLink(fileResource.fsPath, (error: Error, { stat, isSymbolicLink }) => {
	isSymbolicLink = isSymbolicLink;
	this(null, stat);
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did not do this due to name clash of the isSymoblicLink variable a couple of lines up. Decided to leave like it is.

isSymbolicLink = statAndIsLink.isSymbolicLink;
this(null, statAndIsLink.stat);
});
},

function countChildren(this: any, fsstat: fs.Stats): void {
Expand All @@ -1254,6 +1255,7 @@ export class StatResolver {
const childStat: IFileStat = {
resource: fileResource,
isDirectory: fileStat.isDirectory(),
isSymbolicLink,
name: file,
mtime: fileStat.mtime.getTime(),
etag: etag(fileStat),
Expand Down Expand Up @@ -1293,4 +1295,4 @@ export class StatResolver {
});
});
}
}
}
4 changes: 2 additions & 2 deletions src/vs/workbench/services/files/test/node/resolver.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function create(relativePath: string): StatResolver {
let absolutePath = relativePath ? path.join(basePath, relativePath) : basePath;
let fsStat = fs.statSync(absolutePath);

return new StatResolver(uri.file(absolutePath), fsStat.isDirectory(), fsStat.mtime.getTime(), fsStat.size, void 0);
return new StatResolver(uri.file(absolutePath), fsStat.isSymbolicLink(), fsStat.isDirectory(), fsStat.mtime.getTime(), fsStat.size, void 0);
Copy link
Member

Choose a reason for hiding this comment

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

@isidorn can we have a test for this too? it looks like node.js allows to create symlinks (fs.symlink).

}

function toResource(relativePath: string): uri {
Expand Down Expand Up @@ -183,4 +183,4 @@ suite('Stat Resolver', () => {
})
.done(() => done(), done);
});
});
});
1 change: 1 addition & 0 deletions src/vs/workbench/services/textfile/common/textFileEditorModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
mtime: content.mtime,
etag: content.etag,
isDirectory: false,
isSymbolicLink: false,
children: void 0
};
this.updateLastResolvedDiskStat(resolvedStat);
Expand Down