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

Incorrect usage of URI#path.toString() on Windows #10830

Closed
1 of 12 tasks
msujew opened this issue Mar 2, 2022 · 3 comments · Fixed by #11180
Closed
1 of 12 tasks

Incorrect usage of URI#path.toString() on Windows #10830

msujew opened this issue Mar 2, 2022 · 3 comments · Fixed by #11180
Assignees
Labels
filesystem issues related to the filesystem OS/Windows issues related to the Windows OS

Comments

@msujew
Copy link
Member

msujew commented Mar 2, 2022

Description

This issue is meant to collect incorrect usages of displaying URI strings on Windows systems. Simply calling URI#path.toString() usually results in incorrectly formatted paths. See #10591. See also the following as an example:

example

Additional Information

  • Operating System: Windows
  • Theia Version: 1.23.0
@tsmaeder
Copy link
Contributor

@msujew what's the problem with calling uri.path.toString()? Can you give an example that shows the problem?

@tsmaeder
Copy link
Contributor

Also, doing a workspace find for ".path.toString()" reveals many more occurrences than the above. How did you come about the list above?

@msujew
Copy link
Member Author

msujew commented May 18, 2022

@tsmaeder, calling URI.path.toString() on posix systems (non-Windows) is completely fine. Doing the same on Windows leads to weird file paths, which are likely not to be accepted by the system:

const uri = new URI("file://c:\\path\\to\\file");
uri.path.toString(); // yields "/c:/path/to/file

Although / are fine on Windows, the leading slash isn't really valid. This also leads to issues where certain parts of the backend operate on the direct fs based path presentation, while some others work on the URI based presentation.

Also, doing a workspace find for ".path.toString()" reveals many more occurrences than the above. How did you come about the list above?

Not all path.toString() calls are directly used. Most of the time we do something like this:

const path = uri.path.toString();
... // somewhere later
const uriPath = new URI(path);

Which is fine, but once we start interaction with the file system or displaying these paths to the user, we need to convert them to the correct OS presentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
filesystem issues related to the filesystem OS/Windows issues related to the Windows OS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants