-
Notifications
You must be signed in to change notification settings - Fork 37.3k
Enable search of local Typescript Language Service Plugins (fix #45856) #45858
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
Conversation
mjbvz
left a comment
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.
Looking good. Just a few small changes
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.
Should be array contents comparison
extensions/typescript/package.json
Outdated
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.
Add "isExecutable": true here. This prevents this from ever being used as a workspace setting
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.
Why do you think it should be a user-only setting?
Specifying pluginPaths on workspace level can be quite useful, especially in conjunction with relative paths. In this case plugins configuration can be committed to a source control and shared between team members.
If this is this because of the security concerns, I don't believe this is a more dangerous option than ability to specify typescript.tsdk on a workspace-level.
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.
Yes it is for security reasons. Tsdk asks for user confirmation before executing any code in the workspace but that flow is complicated so I don't want to duplicate it here. Instead we should just limit these this to be a user only setting
|
@mjbvz Changes have been made according to your comments. |
f5c8e12 to
9c43780
Compare
…discovery locations
…, setting made executable
|
Thanks! On track for the March vscode release |
Proposed change for #45856. Added option to discover plugins in project-local node modules