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

Add URI.joinPath() API #9422

Merged
merged 2 commits into from
May 7, 2021
Merged

Conversation

tsmaeder
Copy link
Contributor

@tsmaeder tsmaeder commented May 3, 2021

What it does

This is another go at "vscode.Uri.joinPath is not a function" #8752.

The approach (like in the original PR from @svor) is to derive our URI implementation from the vscode-uri implementation we use elsewhere in Theia. In order for this to work, we do the following:

  1. Create only instances of our own class inside the plugin host process
  2. Whenever we transfer a vscode-uri into the plugin host process, we revive it as an instance of our own class
  3. Whenever we transfer and instance of our own class outside of the plugin host process, we marshal it as a vscode-uri.

In order to achieve this, I've made the replacer/reviver in rpc-protocol.ts pluggable.

How to test

Same as #9199, but make sure that the errors found post-merge are not occurring (in particular with the Timeline view.

Review checklist

Reminder for reviewers

Signed-off-by: tsmaeder <tmader@redhat.com>
Signed-off-by: tsmaeder <tmader@redhat.com>
@tsmaeder
Copy link
Contributor Author

tsmaeder commented May 5, 2021

@caseyflynn-google since you're waiting for this bugfix...tag, you're it.

Copy link
Contributor

@caseyflynn-google caseyflynn-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested joinPath and verified timeline, works great!

theia-pr-9422-verification

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants