-
Notifications
You must be signed in to change notification settings - Fork 28.7k
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.
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
relativeFileDirname
gives empty string when the Dirname equals to current working directory
#108588
Conversation
…urrent working directory Fixes #108437
@@ -278,7 +278,8 @@ export class AbstractVariableResolverService implements IConfigurationResolverSe | |||
} | |||
const dirname = paths.dirname(getFilePath()); | |||
if (folderUri || argument) { | |||
return paths.relative(this.fsPath(getFolderUri()), dirname); | |||
const relative = paths.relative(this.fsPath(getFolderUri()), dirname); | |||
return relative.length === 0 ? '.' : relative; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We used to call paths.normalize(relative)
, which does the conversion from empty string to '.'. Since it isn't safe to call normalize
on a path that could come from a remote, I removed it. In this case though, we still need the '.' instead of empty string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's potential for refactoring here as fsPath
and path.relative
should not be used on remote URIs.
But for the fix release the change is minimal and good.
@@ -278,7 +278,8 @@ export class AbstractVariableResolverService implements IConfigurationResolverSe | |||
} | |||
const dirname = paths.dirname(getFilePath()); | |||
if (folderUri || argument) { | |||
return paths.relative(this.fsPath(getFolderUri()), dirname); | |||
const relative = paths.relative(this.fsPath(getFolderUri()), dirname); | |||
return relative.length === 0 ? '.' : relative; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's potential for refactoring here as fsPath
and path.relative
should not be used on remote URIs.
But for the fix release the change is minimal and good.
Fixes #108437