-
Notifications
You must be signed in to change notification settings - Fork 419
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
Support plugins configuration in omnisharp.json #1615
Support plugins configuration in omnisharp.json #1615
Conversation
thanks - overall this is a good idea and a welcome addition. This would mean that we need to refactor the code a bit to perhaps delay the decision about plugin discovery or move the options bootstrapping up. |
I'm with @filipw I like the idea but the solution feels wrong somehow. I'm going to have to circle back to this, this weekend after my local code camp. |
Good feedback! Switched to using strongly typed object for declaring locations for plugins. |
@filipw @david-driscoll Curious what you guys think of the latest changes. |
thanks a lot - it will be reviewed carefully as soon as there is some free time. |
@filipw a gentle ping since it's been few weeks. thanks |
sorry! I will do my best to review in the coming days! 🙏 |
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.
LGTM - thanks!
any thoughts? @david-driscoll @mholo65 @JoeRobich if not IMHO this is good to merge and a very valuable feature |
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.
Great work. A minor nit/question about the logging in the assembly loader extension.
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.
LGTM
…ILogger" This reverts commit 65899df.
LGTM |
thanks! |
Locations for OminSharp’s plugins can currently be configured only via –plugin command line argument. But when OminSharp is activated as a part of ms-vscode.csharp extension, other VSCode extensions cannot use this approach to supply their own plugins for OmniSharp. The only exception are code actions which can be configured in omnisharp.json via RoslynExtensionsOptions. The proposed changes enable OmniSharp to discover any plugin using the same technique via new Plugins section in omnisharp.json, i.e.:
An example of VSCode extension that provides plugins via omnisharp.json is Roslynator which does it via RoslynExtensionsOptions.