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

contextUri.path wrongly uppercased on Windows #116298

Closed
cyrilletuzi opened this issue Feb 10, 2021 · 4 comments
Closed

contextUri.path wrongly uppercased on Windows #116298

cyrilletuzi opened this issue Feb 10, 2021 · 4 comments
Assignees
Labels
file-explorer Explorer widget issues under-discussion Issue is under discussion for relevance, priority, approach
Milestone

Comments

@cyrilletuzi
Copy link

  • VS Code Version: 1.53.1
  • OS Version: macOS 11.2

Hi, I'm the author of the Angular schematics extension (400 000 users).

Basically, it's a graphic interface for the Angular CLI (ie. the generation of component files), so the user right-click on a folder in the Explorer menu where he/she wants to generate, then the extension retrieves the folder path and pre-fill the component path.

To do so, I basically do this:

import * as path from 'path';

path.posix.relative(workspaceFolder.uri.path, contextUri.path);

where contextUri is the argument passed by VS Code when the user right-clicked a folder and chose "Angular: Generate something" in the context menu from the extension.

Issue is on Windows, contextUri.path is inconsistent:

  • sometimes the drive letter is lowercased (/c:/Users/Elmo/angular-project/right-clicked-folder), which is what is expected as all other VS Code paths are like that (for example workspaceFolder.uri.path)
  • sometimes the drive letter is uppercased (/C:/Users/Elmo/angular-project/right-clicked-folder), which causes path.posix.relative() to give the wrong result

When testing myself, the second and wrong scenario happens only in a multi-workspaces setup. But the user of the extension who reported the issue first is telling he is in a single workspace setup.

Steps to Reproduce:

On Windows:

  1. Generate a random Angular project (to be able to see the extension logs, as it only activates in Angular projects):
  • npm install @angular/cli -g
  • ng new (options asked don't matter)
  1. Generate a second random Angular project (to be able to test in a multi-workspaces setup):
  • ng new (options asked don't matter)
  1. Clone the extension:
  • git clone https://github.com/cyrilletuzi/vscode-angular-schematics
  • cd vscode-angular-schematics
  • git checkout windowsissuewithcontexturi
  • npm install
  1. Open the extension project in VS Code
  2. Launch a debug session (just click "Run" in the debug view)
  3. In the debug window:
  • open the first Angular project
  • right-click on any folder or file in the Explorer menu
  • choose "Angular: generate a component" in the context menu
  1. Go back to the extension project window and check the Debug console: the path should be correct (/c:/Users/Elmo/angular-project/right-clicked-folder)
  2. Go back to the debug window:
  • File > Add a new folder to workspace > choose the second Angular project
  • right-click on any folder or file in the Explorer menu
  • choose "Angular: generate a component" in the context menu
  1. Go back to the extension project window and check the Debug console: the path should be incorrect (/C:/Users/Elmo/angular-project/right-clicked-folder)

Does this issue occur when all extensions are disabled?: Yes

@isidorn
Copy link
Contributor

isidorn commented Feb 10, 2021

@cyrilletuzi so you want the drive letter to be normalized for explorer contributed context commands.
This makes sense, however we will not do it.
It might break a lot of extensions and nobody complained in these 5 years. Explorer simply passes the URI along, it does not change the drive letter.

For now leaving this open to gather more feedback.

@isidorn isidorn added file-explorer Explorer widget issues under-discussion Issue is under discussion for relevance, priority, approach and removed new release labels Feb 10, 2021
@isidorn isidorn added this to the Backlog milestone Feb 10, 2021
@cyrilletuzi
Copy link
Author

cyrilletuzi commented Feb 10, 2021

so you want the drive letter to be normalized for explorer contributed context commands

No, it's not what I ask. I ask nothing, I report the issue of vscode.Uris path (and fsPath) being inconsistent in the case of contextUri:

  • it's not normal that Node path.relative() fails with 2 paths coming from the same API and on the same OS, it definitively means that one of the path is wrongly formatted,
  • it's also not normal that the exact same API (contextUri.path) returns a different value for the exact same right-clicked folder, depending on unknown conditions.

Explorer simply passes the URI along

I'm quite sure it's not exact. From what I understand, vscode.Uris contain:

  • .fsPath: the OS-specific path (so C:\path\to on Windows, /usr/path/to on Linux)
  • .path: the path in Linux format (so /C:/path/to on Windows, /usr/path/to on Linux)

While fsPath is indeed probably just a raw value, I'm quite sure path is derived and formatted from fsPath.

So maybe the issue is not in contextUri management itself but in some cases where path is not derived correctly from fsPath, but still there is an issue somewhere.

@isidorn
Copy link
Contributor

isidorn commented Feb 11, 2021

fyi @jrieken as the owner of our URI class

@cyrilletuzi
Copy link
Author

Closing the issue as it has not been taken into account and is now lost in the void, but the issue is still there.

@github-actions github-actions bot locked and limited conversation to collaborators May 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
file-explorer Explorer widget issues under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

2 participants