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.

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

workspace.asRelativePath on windows returns modified string for failure #33709

Closed
vrachels opened this issue Sep 1, 2017 · 4 comments
Closed
Assignees
Labels
api bug Issue identified by VS Code Team member as probable bug verified Verification succeeded

Comments

@vrachels
Copy link

vrachels commented Sep 1, 2017

  • VSCode Version: Code 1.15.1 (41abd21, 2017-08-16T18:05:38.342Z)
  • OS Version: Windows_NT ia32 10.0.15063
  • Extensions:
Extension Author (truncated) Version
better-toml bun 0.2.0
tslint eg2 0.17.0
debugger-for-chrome msj 3.2.0
autoimport ste 1.2.2

initially found with VSCode Version: Code - Insiders 1.16.0-insider (aa42e6e, 2017-09-01T15:40:17.463Z)

workspace.asRelativePath returns unexpected values in two ways:

workspace.asRelativePath(workspace.rootPath) does not return '' or './'

ok, so the documentation says

...or when the path * is not a child of that folder, the input is returned.

I'll give that part of it a pass, as the root is technically not a child.

It does, however, return a modified string, instead of the input: the input's drive backslash is returned as a forward slash.

const wksRoot = workspace.rootPath;  // e:\foo
const rel = workspace.asRelativePath(wksRoot) // e:/foo

This is not specific to the root, however. Any \ gets converted to /

const rel = workspace.asRelativePath('D:\\BAR\\FOO') // d:/bar/foo

expected behavior is to actually return the input as specified, so the following code would be valid:

const rel = workspace.asRelativePath(testPath);
if (testPath === rel) { // do non-relative path things }

having to do a separate test for (testPath === workspace.rootPath) is not ideal, either. How is this intended to be handled for the multi-root feature? Will it still return the testPath?
Perhaps now would be a great time to make this feature more usable, and create a better function that clarifies and makes more usable the different use cases, rather than the current solution of having asRelativePath report different things depending on the project setup (yuk!) and environment

suggested api:

export interface IRelativePathResult {
  relativePath: string,
  foundRoot: string
}
export function getRelativePath(pathOrUri: string | Uri): IRelativePathResult;

result is null for path not related
result.relativePath is '' if the input was a root path, or the full relative path
result.foundRoot is the closest root (or the input if exact match)

ideally:

if(typeof inputPath === 'string') {
  assert (result.foundRoot + result.relativePath === inputPath);
}

but that could be a stretch goal

@vrachels
Copy link
Author

vrachels commented Sep 1, 2017

-insiders label please

@jrieken jrieken self-assigned this Sep 4, 2017
@jrieken jrieken modified the milestones: August 2017, September 2017 Sep 4, 2017
@jrieken
Copy link
Member

jrieken commented Sep 4, 2017

Yeah, this has been introduced via 588b14b and I am not really sure why...

@jrieken
Copy link
Member

jrieken commented Sep 4, 2017

How is this intended to be handled for the multi-root feature?

It depends on your use-case of which I would like to learn more. Also there is getWorkspaceFolder which gives you undefined when called with a workspace folder

@vrachels
Copy link
Author

vrachels commented Sep 5, 2017

I did see that function, and a reference to a (semi-) similar issue at #31553, but there is still some usability issues, either way. getWorkspaceFolder of course doesn't get the relative path, and how does the getRelativePath behave with a workspace root as the input in either case?

@jrieken jrieken added the bug Issue identified by VS Code Team member as probable bug label Sep 8, 2017
jrieken added a commit that referenced this issue Sep 8, 2017
@jrieken jrieken closed this as completed Sep 8, 2017
@chrmarti chrmarti added the verified Verification succeeded label Sep 29, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants