-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Make CompletionOptions global #59117
Conversation
f1e9e26
to
e2e335c
Compare
src/Features/Core/Portable/Completion/CompletionOptionsMetadata.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/Completion/CompletionOptionsMetadata.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/Completion/CompletionOptionsMetadata.cs
Outdated
Show resolved
Hide resolved
src/Features/LanguageServer/Protocol/Handler/SignatureHelp/SignatureHelpHandler.cs
Show resolved
Hide resolved
src/Features/LanguageServer/ProtocolUnitTests/Workspaces/LspWorkspaceManagerTests.cs
Show resolved
Hide resolved
src/VisualStudio/CSharp/Impl/Options/IntelliSenseOptionPageControl.xaml.cs
Outdated
Show resolved
Hide resolved
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. have some questions i'd like some clarity on just to make sure my understanding is correct.
7354476
to
882eb24
Compare
882eb24
to
39f4fab
Compare
a9cc7a2
to
7a090c9
Compare
/// TODO: Options are currently explicitly listed since <see cref="OptionKey"/> is not serializable. | ||
/// https://github.com/dotnet/roslyn/issues/59267 |
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.
💡 I recommend resolving this as won't fix. New integration tests already support OptionKey
directly, and old integration tests are being actively ported to the new library.
Removes CompletionOptions from Solution snapshot. The options are now only stored in global options.
Establishes a new pattern: Option definitions move from being nested as
XyzOptions.Metadata
to a separate typeXyzOptionsStorage
(CompilationOptionsStorage
in this case) that also defines an extension methodGetXyzOptions
onIGlobalOptionService
used to fetch them. This type is to be moved to client code (editor features).This completes the separation of options from their metadata and storage.
While working on this refactoring I realized that certain scenarios were broken by previous completion options changes. When 3rd party code passes OptionSet containing their custom options into our public completion APIs these options got lost and were not available to their custom CompletionProvider. This PR fixes that by passing 3rd party options through.