Node Explorer: follow symbolic links to directories #249
Conversation
Signed-off-by: Naman Sood <mail@nsood.in>
src/sftp.ts
Outdated
| if (!result.startsWith('/')) { | ||
| // split path by separator, slice away the last bit to get the parent directory, | ||
| // append the relative path, and join it back together | ||
| result = [...path.split('/').slice(0, -1), result].join('/'); |
There was a problem hiding this comment.
Given the /, does this work for "shortucts" on windows? Why not use path.dirname?
There was a problem hiding this comment.
I did not know path.dirname existed. 😅 I can switch to using that now.
I'm going to guess that this doesn't work with shortcuts on Windows, since those are a completely different mechanism from symbolic links. Windows also has symbolic links and I would guess that this would work with those (but I have no way of checking, since I don't own a Windows PC).
The path here is a URI, so it should use / regardless of platform.
There was a problem hiding this comment.
Update: this doesn't work for anything on a Windows remote, because Tailscale SSH does not support Windows at the moment. 😅
src/sftp.ts
Outdated
| @@ -13,13 +13,30 @@ export class Sftp { | |||
| return this.sftpPromise; | |||
| } | |||
|
|
|||
| async readSymbolicLink(path: string): Promise<string> { | |||
There was a problem hiding this comment.
Can we also add some unit tests for this function?
Signed-off-by: Naman Sood <mail@nsood.in>
Signed-off-by: Naman Sood <mail@nsood.in>
Signed-off-by: Naman Sood <mail@nsood.in>
Fixes #209.
The biggest change in this PR is that we'll be treating
vscode.FileTypeas a bitfield now, rather than just checking for equality, since the SFTP file provider is modified to return symlinks asSymbolicLink | FileorSymbolicLink | Directoryrather than justSymbolicLink.Also, should wait for @shayne (or someone else) to confirm that SFTP to a Windows host still uses
/as the path separator before merging (unfortunately I don't have a Windows machine to test).