Allow OmniSharp to provide ImplementType Options #75312
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
|
@tmat Could you take a look? You know this area better than most. |
JoeRobich
force-pushed
the
dev/jorobich/update-o#-ea
branch
from
bbc2cf4 to
8f1a5c6
Compare
| <ItemGroup> | ||
| <InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.CSharp" /> | ||
| <InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.UnitTests" /> | ||
| <!-- | ||
| ⚠ ONLY OMNISHARP ASSEMBLIES MAY BE ADDED HERE ⚠ | ||
| --> | ||
| <InternalsVisibleTo Include="OmniSharp.Host" Key="$(OmniSharpKey)" /> |
There was a problem hiding this comment.
O# keeps the workspace options updated in this assembly.
|
|
||
| namespace Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.Options; | ||
|
|
||
| internal sealed record class OmniSharpEditorConfigOptions |
There was a problem hiding this comment.
For the time being these are the only fallback options that have matching O# options
src/Features/ExternalAccess/OmniSharp/Options/OmniSharpEditorConfigOptions.cs
Outdated
Show resolved
Hide resolved
| var oldFallbackOptions = oldSolution.FallbackAnalyzerOptions; | ||
| oldFallbackOptions.TryGetValue(LanguageNames.CSharp, out var csharpFallbackOptions); | ||
|
|
||
| var changedOptions = DetermineChangedOptions(csharpFallbackOptions, editorConfigOptions); |
There was a problem hiding this comment.
Do we need to determine changed options? Why not set all the O# options unconditionally?
There was a problem hiding this comment.
No, that simplifies a lot. Thanks!
|
@tmat updated! Thanks |
src/Features/ExternalAccess/OmniSharp/Options/OmniSharpSolutionAnalyzerConfigOptionsUpdater.cs
Outdated
Show resolved
Hide resolved
|
|
||
| namespace Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.Options; | ||
|
|
||
| using Workspace = CodeAnalysis.Workspace; |
There was a problem hiding this comment.
Workspace conflicts because there is a Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.Workspace namespace.
src/Features/ExternalAccess/OmniSharp/Options/OmniSharpSolutionAnalyzerConfigOptionsUpdater.cs
Outdated
Show resolved
Hide resolved
| { | ||
| var configName = option.Definition.ConfigName; | ||
| var configValue = option.Definition.Serializer.Serialize(value); | ||
| builder.Add(configName, configValue); |
There was a problem hiding this comment.
Should this not replace the value if it already exists?
| static void AddOption<T>( | ||
| PerLanguageOption2<T> option, | ||
| T value, | ||
| ImmutableDictionary<string, string>.Builder builder) |
There was a problem hiding this comment.
nit: you can make this function non-static and remove builder param.
Applies fixups for feedback left on #75312
This should allow OmniSharp to provide ImplementType Options at the workspace level. Something similar to their CSharpFormattingWorkspaceOptionsProvider. Will be inserted into O# as part of OmniSharp/omnisharp-roslyn#2642