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

Issue Opening Folder Using vscode.openFolder #55891

Closed
allileong opened this issue Aug 6, 2018 · 16 comments
Closed

Issue Opening Folder Using vscode.openFolder #55891

allileong opened this issue Aug 6, 2018 · 16 comments
Assignees
Labels
api bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release verified Verification succeeded
Milestone

Comments

@allileong
Copy link

allileong commented Aug 6, 2018

  • VSCode Version: 1.26.0-Insider (This issue is happening in Insiders only)
  • OS Version: macOS 10.13.6

While running a custom extension, I execute:

vscode.commands.executeCommand(
     'vscode.openFolder',
     vscode.Uri.parse(
        <some-local-path>
     )
);

I get a 'Path does not exist error', even though the path exists.
screen shot 2018-08-06 at 9 42 14 am

Steps to Reproduce:

  1. Run a custom extension
  2. Execute the code snippet above
@ramya-rao-a
Copy link
Contributor

cc @bpasero

@ramya-rao-a ramya-rao-a added the candidate Issue identified as probable candidate for fixing in the next release label Aug 6, 2018
@ramya-rao-a ramya-rao-a added this to the July 2018 milestone Aug 6, 2018
@ramya-rao-a ramya-rao-a removed the candidate Issue identified as probable candidate for fixing in the next release label Aug 6, 2018
@ramya-rao-a ramya-rao-a modified the milestones: July 2018, August 2018 Aug 6, 2018
@ramya-rao-a
Copy link
Contributor

@allileong It works if you prefix the local path with file:/ for me. For example: file://Users/me/hello. Can you try that?

@jrieken In the stable bits of 1.25.1, this would work even without the prefix of the scheme. I am not sure if the api for Uri changed or the one for vscode.openFolder command. Regardless, this would be a breaking changes for extension authors.

@ramya-rao-a ramya-rao-a added the candidate Issue identified as probable candidate for fixing in the next release label Aug 6, 2018
@ramya-rao-a ramya-rao-a added this to the July 2018 milestone Aug 6, 2018
@bpasero
Copy link
Member

bpasero commented Aug 6, 2018

@aeschli maybe due to the changes for supporting URIs on the main side?

/cc @sandy081

@bpasero bpasero added the important Issue identified as high-priority label Aug 6, 2018
@aeschli
Copy link
Contributor

aeschli commented Aug 6, 2018

The vscode.openFolder command API hasn't changed. The second argument always was a URI.
The difference is that now we use that URI to open the folder, while before we used only uri.fsPath.

That surfaces the bug in the user code:
vscode.Uri.parse(<some-local-path>) is wrong.
It needs to be vscode.Uri.file(<some-local-path>).

The error message is confusing. The error message is shown when trying to open a worbench window on an invalid URI (in this case, a URI without scheme).
I suggest we improve that.

@allileong
Copy link
Author

Hi @ramya-rao-a , prefixing the local path with file:/ gets me past the issue.

@aeschli
Copy link
Contributor

aeschli commented Aug 6, 2018

@allileong Please use vscode.Uri.file. Thats the proper way to create a URI from a file path that works on all platforms.

@kieferrm
Copy link
Member

kieferrm commented Aug 6, 2018

@aeschli what you're saying is that our implementation of vscode.URI.parse behaves incorrectly. It's documented as:

		/**
		 * Create an URI from a string. Will throw if the given value is not
		 * valid.
		 *
		 * @param value The string value of an Uri.
		 * @return A new Uri instance.
		 */
		static parse(value: string): Uri;

but for local file path inputs it creates URIs without a mandatory scheme component, i.e. invalid URIs.

@jrieken
Copy link
Member

jrieken commented Aug 7, 2018

you're saying is that our implementation of vscode.URI.parse behaves incorrectly

I don't think he is saying that. The issue is this: Uri.parse('c:\foo\bar') is different from Uri.file('c:\foo\bar'). The sample code is vscode.Uri.parse(<some-local-path>) which is incorrect, when you have path you must use Uri.file

@jrieken
Copy link
Member

jrieken commented Aug 7, 2018

but for local file path inputs it creates URIs without a mandatory scheme component, i.e. invalid URIs.

We don't throw when uri components are missing, we only do little validation e.g. checking authority and path constraints etc

@joaomoreno joaomoreno added the bug Issue identified by VS Code Team member as probable bug label Aug 7, 2018
@jrieken
Copy link
Member

jrieken commented Aug 7, 2018

I have experimented with enforcing that URIs must have a scheme... Un-commenting this code yields in many, many test failures and while the product starts we will likely see failure here and there. So, iff we want to support scheme-enforcement a coordinated effort is needed.

@aeschli
Copy link
Contributor

aeschli commented Aug 7, 2018

We now throw an exception when invoking vscode.openFolder with an invalid (schema-less) URI.

@aeschli
Copy link
Contributor

aeschli commented Aug 7, 2018

I change the fix to treat the missing schema gracefully. The URI without scheme is converted to a file:// URI. That change only applies to vscode.openFolder.

Extensions are advised to not rely on the workaround but change vscode.Uri.parse to vscode.Uri.file. Note that vscode.Uri.parse(<filename>) doesn't (and hasn't) work on Windows as drive letters (c:/) are treated as URI schemes and will not be part anymore of uri.fsPath.

@chrmarti chrmarti added the verified Verification succeeded label Aug 8, 2018
@chrmarti
Copy link
Contributor

chrmarti commented Aug 8, 2018

@aeschli The warning message doesn't expand ${uri} because it is not in a template string. Verified otherwise.

@aeschli
Copy link
Contributor

aeschli commented Aug 8, 2018

@chrmarti Good catch, I added a fix. I also improved the fix to use URI.file(uri.toString()) as alternative URL (then it also works on windows).

@aeschli
Copy link
Contributor

aeschli commented Aug 8, 2018

@chrmarti If you can verify again that would be great

aeschli added a commit that referenced this issue Aug 8, 2018
aeschli added a commit that referenced this issue Aug 8, 2018
@chrmarti chrmarti added verified Verification succeeded and removed verified Verification succeeded labels Aug 9, 2018
@sandy081
Copy link
Member

Thanks @aeschli for the fix

@vscodebot vscodebot bot locked and limited conversation to collaborators Sep 21, 2018
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 candidate Issue identified as probable candidate for fixing in the next release verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

9 participants