-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 theia.Uri.joinPath and theia.PluginContext.extensionUri API #9199
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@svor please squash to a single commit before we can merge the changes 👍
Signed-off-by: svor <vsvydenk@redhat.com>
done |
@vince-fugnitto is this PR good enough to merge? |
@svor just a question, can we re-use |
@vince-fugnitto in case we update |
Thanks for following up, it's probably something we should look into updating (to a version supported by our monaco) rather than add the wrapper around it for the plugin system. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally the pull-request works well 👍
@vince-fugnitto Thanks for your review, after updating monaco all these problem related to Uri should be resolved but before that I don't see another way except adding |
I'm fine with the approach for now :) it is definitely something to think about when we do the upgrade thank you! |
@vince-fugnitto @RomanNikitenko I don't have permissions to merge the PR, who is the best person to do that or we need more reviewers here? |
@svor Looks like your commit caused this bug #9221. The timeline provider which is implemented in the vscode git plugin doesn't go to this condition: https://github.com/microsoft/vscode/blob/3767f97bc32b815802ceeac007a142c56581ca8b/extensions/git/src/model.ts#L423 |
@vinokurig nice catch, it seems like this is due to the way the From there it is not hard to imagine landing in the scenario where
vscode-uri is still used... |
@vinokurig @marechal-p thank you for your comments, I tried to reuse the new extended class everywhere in |
PR to revert changes: #9222 |
What it does
extensionUri
into PluginContext APIjoinPath
into Uri API (vscode.Uri.joinPath is not a function #8752)How to test
The command for testing should just display a notification with info about path to package.json file.
Review checklist
Reminder for reviewers