Formatting options fallback and Document extensions#60803
Conversation
fc5d3da to
95559cf
Compare
|
I haven't looked at the pr yet. However my general concern is around how a particular piece of coffee knows which overload to use, and if there are problems if they use the wrong one. For example:
What if you're not in the codestyle layer? Can you still use these? Should you? Would it be bug if you used the wrong one? |
You can use any of the overloads that are available in the a particular layer. They achieve the same. The overloads are mostly for convenience, so that starting from a Document you can easily get the options needed. The only decision to make is whether it makes sense to read the options eagerly (using XyzOptions) or whether they need to be read lazily (using a XyzOptionsProvider). The former is preferred in code that operates on a single document. Then you can parameterize that feature with specific options and read them upfront. If a feature operates on multiple documents that potentially come from different projects then you have to use the provider since we do not want to load specific language binaries upfront if they won't actually be used. If there is a performance concern with reading options from editorconfig documents upfront (perhaps a feature only needs the options rare cases) then you can also use the provider pattern. |
|
That sounds great. Thanks for the explanation. Will review tomorrow! |
95559cf to
2d43690
Compare
dcbca1a to
d829531
Compare
d829531 to
6009f81
Compare
src/Analyzers/Core/Analyzers/Helpers/AnalyzerOptionsExtensions.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/CSharp/AddImports/CSharpAddImportPlacementOptionsStorage.cs
Show resolved
Hide resolved
src/EditorFeatures/VisualBasic/AddImports/VisualBasicAddImportPlacementOptionsStorage.vb
Show resolved
Hide resolved
| lineFormatting:=globalOptions.GetLineFormattingOptions(LanguageNames.VisualBasic), | ||
| separateImportDirectiveGroups:=globalOptions.GetOption(GenerationOptions.SeparateImportDirectiveGroups, LanguageNames.VisualBasic)) | ||
| End Function | ||
|
|
| using Microsoft.CodeAnalysis.Remote; | ||
| using Microsoft.CodeAnalysis.Text; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.EncapsulateField |
|
|
||
| namespace Microsoft.CodeAnalysis.ConvertTupleToStruct | ||
| { | ||
| internal interface IRemoteConvertTupleToStructCodeRefactoringService |
| } | ||
|
|
||
| #if TODO // Replace the below specialization with a call to a generic impl once https://github.com/microsoft/vs-streamjsonrpc/issues/789 is fixed | ||
| private CodeCleanupOptionsProvider GetClientOptionsProvider(RemoteServiceCallbackId callbackId) |
There was a problem hiding this comment.
Creates options provider that fetches options from the client and caches them for a given language.
| } | ||
|
|
||
| #if TODO // Replace the below specialization with a call to a generic impl once https://github.com/microsoft/vs-streamjsonrpc/issues/789 is fixed |
There was a problem hiding this comment.
Creates options provider that fetches options from the client and caches them for a given language.
| } | ||
|
|
||
| #if TODO // Replace the below specialization with a call to a generic impl once https://github.com/microsoft/vs-streamjsonrpc/issues/789 is fixed |
There was a problem hiding this comment.
Creates options provider that fetches options from the client and caches them for a given language.
| @@ -74,4 +81,43 @@ public CodeActionOptions() | |||
| #endif | |||
|
|
|||
| internal delegate CodeActionOptions CodeActionOptionsProvider(HostLanguageServices languageService); | |||
|
|
|||
| internal static class CodeActionOptionsProviders | |||
There was a problem hiding this comment.
Defines extensions methods that make fetching options from Document and converting between providers convenient.
| internal static class CSharpSyntaxFormattingOptionsStorage | ||
| { | ||
| [ExportLanguageService(typeof(ISyntaxFormattingOptionsStorage), LanguageNames.CSharp), Shared] | ||
| private sealed class Service : ISyntaxFormattingOptionsStorage |
There was a problem hiding this comment.
Implements language specific storage. This allows common code to get language specific options instance from global options
| => GetCSharpSyntaxFormattingOptions(globalOptions); | ||
| } | ||
|
|
||
| public static CSharpSyntaxFormattingOptions GetCSharpSyntaxFormattingOptions(this IGlobalOptionService globalOptions) |
There was a problem hiding this comment.
Reads C# specific formatting options from global options
|
|
||
| internal static class SyntaxFormattingOptionsStorage | ||
| { | ||
| public static ValueTask<SyntaxFormattingOptions> GetSyntaxFormattingOptionsAsync(this Document document, IGlobalOptionService globalOptions, CancellationToken cancellationToken) |
There was a problem hiding this comment.
Adds convenience extension methods to read formatting options from document with fallback to global options.
1d205c8 to
097656f
Compare
OpenFileOnly Simplifier options defaults StreamJsonRpc bug workaround Make LineFormattingOptions class to avoid serialization issues
c4f7378 to
9a6bfaf
Compare
…ures/semi-auto-props * upstream/main: (266 commits) Pass fallback options (#60803) Rename `CodeStyleHostLanguageServices.cs.cs` to `CodeStyleHostLanguageServices.cs` (#60955) Allow VSMac to access LSP options (#60943) Add configs for 17.3 branch and update main version (#60942) Restore nugetKind config to publish data Remove unnecessary publish data config Remove non-servicing 15.x publish config Let 'arcade' packageFeeds imply all feeds are 'arcade' Remove non-servicing branches from our PublishData Handle unexpected keyword rather than identifier for lambda parameter name (#60825) Correctly return E_NOTIMPL when asked for file code models for non-source Move the resources to top (#60899) Add back deprecated packages to arcade publishing config. Simplify Rename parameter Simplify visibility logic in tagger Simplify visibility logic in tagger Try out some fixes Update StructConstructorTests.cs Use more descriptive variable name ...
…ures/required-members * upstream/main: (156 commits) Pass fallback options (#60803) Rename `CodeStyleHostLanguageServices.cs.cs` to `CodeStyleHostLanguageServices.cs` (#60955) Allow VSMac to access LSP options (#60943) Add configs for 17.3 branch and update main version (#60942) Restore nugetKind config to publish data Remove unnecessary publish data config Remove non-servicing 15.x publish config Let 'arcade' packageFeeds imply all feeds are 'arcade' Remove non-servicing branches from our PublishData Handle unexpected keyword rather than identifier for lambda parameter name (#60825) Correctly return E_NOTIMPL when asked for file code models for non-source Move the resources to top (#60899) Add back deprecated packages to arcade publishing config. Simplify Rename parameter Simplify visibility logic in tagger Simplify visibility logic in tagger Try out some fixes Update StructConstructorTests.cs Use more descriptive variable name ...
…o forbid-new * upstream/features/required-members: (156 commits) Pass fallback options (dotnet#60803) Rename `CodeStyleHostLanguageServices.cs.cs` to `CodeStyleHostLanguageServices.cs` (dotnet#60955) Allow VSMac to access LSP options (dotnet#60943) Add configs for 17.3 branch and update main version (dotnet#60942) Restore nugetKind config to publish data Remove unnecessary publish data config Remove non-servicing 15.x publish config Let 'arcade' packageFeeds imply all feeds are 'arcade' Remove non-servicing branches from our PublishData Handle unexpected keyword rather than identifier for lambda parameter name (dotnet#60825) Correctly return E_NOTIMPL when asked for file code models for non-source Move the resources to top (dotnet#60899) Add back deprecated packages to arcade publishing config. Simplify Rename parameter Simplify visibility logic in tagger Simplify visibility logic in tagger Try out some fixes Update StructConstructorTests.cs Use more descriptive variable name ...
Establishes a common pattern to read options that are stored in editorconfig and for which IDE also provides defaults (fallback options).
Adds
GetXyzOptionsAsync(...)extension methods toDocumentfor each options type (SyntaxFormattingOptions,SimplifierOptions,CodeCleanupOptions, etc.). These methods takefallbackOptions-- IDE supplied options that are used in case of a specific option is not present in the document's editorconfig option set.fallbackOptionsare passed around as eitherXyzOptionsinstanceXyzOptionsinstance (XyzOptionsProvider) for lazy retrieval of language specific optionsIGlobalOptionServicein editor layer and above,In code style layer the method also takes
ISyntaxFormattingorISimplificationinstance, which is a non-service type that is used as a factory for language specific options. In workspaces and features layers the corresponding language services serve as factories (e.g.ISyntaxFormattingService,ISimplificationService).To facilitate lazy retrieval of language specific options OOP a service that may operate with multiple languages needs to have an RPC callback (
ICallback) that will be called to retrieve options from the client as needed. A couple of helper types and a method are added to make it easier to implement this callback in common cases:Types
RemoteOptionsProvider<TOptions>,IRemoteOptionsCallback<TOptions>, andBrokeredServiceBasemethodValueTask<TOptions> GetClientOptionsAsync<TOptions, TCallbackInterface>(...). The helper caches options retrieved from the client so that we only call to the client once for each language.Follow up issues:
Related MessagePack/StreamJsonRpc issues:
Validation build:
https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/395452
Some RPS failures but it seems only in the infrastructure uploading test results.