VSMac external access for types required to startup the roslyn LSP server#60330
VSMac external access for types required to startup the roslyn LSP server#60330dibarbet merged 2 commits intodotnet:mainfrom
Conversation
|
I think this is the first thing VS Mac will be using through EA 😁 |
sandyarmstrong
left a comment
There was a problem hiding this comment.
Why is the wrapper and vsmac-specific interface necessary? Why not just have vsmac implement/export the interfaces directly?
| /// as an <see cref="ILspLoggerFactory"/> for inclusion in the vsmac composition. | ||
| /// </summary> | ||
| [Export(typeof(ILspLoggerFactory)), Shared] | ||
| internal class VSMacLspLoggerFactoryWrapper : ILspLoggerFactory |
There was a problem hiding this comment.
On the Windows side, this factory is in the VS layer, not the EditorFeatures.Wpf layer. Typically that means the implementation should go in VSMac itself. I know we've discussed the idea of starting to move VSMac's Roslyn integration from VSMac to this repo, but we're not there yet. Whatever the reason is for wanting to implement this in Roslyn instead of VSMac right now, maybe we need a new "VSMac layer" assembly here, so that the purpose of EditorFeatures.Cocoa doesn't diverge too much from EditorFeatures.Wpf.
There was a problem hiding this comment.
Technically there is no reason that the windows impl could not live in the editor features layer, other than at some point the host workspace kind may need to differ between VS and VSMac. At some point in the future I am potentially thinking about unifying all of them as well around a single impl in EditorFeatures, so I think we don't need a new VSMac layer for this right now
this is the external access pattern we've used in the past for teams like F#/TS and has worked well. It allows us to
|
|
Sorry about the conflicts! |
|
/azp run |
|
Azure Pipelines successfully started running 4 pipeline(s). |
Implements two types that are a required part of the mef composition in order to start any of the roslyn LSP servers