Skip to content
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

Move reading HideAdvancedMembers option up #58747

Merged
merged 5 commits into from
Jan 20, 2022
Merged

Conversation

tmat
Copy link
Member

@tmat tmat commented Jan 10, 2022

The goal is to flow all completion options to all services that need them explicitly and remove them from solution snapshot.

Contributes to #58717

@tmat tmat force-pushed the HideAdvancedMembers branch from be02ee5 to 4d443fe Compare January 10, 2022 04:51
@tmat tmat marked this pull request as ready for review January 11, 2022 22:01
@tmat tmat requested review from a team as code owners January 11, 2022 22:01
@tmat tmat mentioned this pull request Jan 18, 2022
@tmat
Copy link
Member Author

tmat commented Jan 18, 2022

@CyrusNajmabadi @sharwell @mavasani @dotnet/roslyn-ide PTAL

context.Options.SearchReferenceAssemblies,
context.Options.HideAdvancedMembers);

var symbolSearchService = options.SearchReferenceAssemblies || searchNuGetPackages
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider leaving the original code here, and just moving the options creation below to where it is first needed.

@tmat tmat enabled auto-merge (squash) January 18, 2022 20:50
@CyrusNajmabadi
Copy link
Member

It could but the reason I didn't do that is because HideAdvancedMembers option is also consumed by other code actions than AddImportsOptions. It'd be odd for these to access it via CodeActionOptions.AddImportsOptions.HideAdvancedMembers.

That makes a ton of sense. Thanks!

@tmat tmat merged commit 52323cb into dotnet:main Jan 20, 2022
@ghost ghost added this to the Next milestone Jan 20, 2022
333fred added a commit to 333fred/roslyn that referenced this pull request Jan 21, 2022
* upstream/main: (1035 commits)
  Add missing header
  Mark IVSTypeScriptFormattingServiceImplementation as optional, but require it in the constructor
  Fix Go To Base for a symbol with a single metadata location (dotnet#58965)
  [EnC] Store entire spans instead of line deltas (dotnet#58831)
  Delete CodeAnalysisRules.ruleset (dotnet#58968)
  Allow xml docs to still be created when 'emit metadata only' is on. (dotnet#57667)
  Fix ParseVBErrorOrWarning (dotnet#47833)
  Update parameter nullability to match implementation
  Ensure CSharpUseParameterNullCheckingDiagnosticAnalyzer works with nullable parameters
  Add tests for issues fixed by previous PR (dotnet#58764)
  Update src/Features/CSharp/Portable/Completion/CompletionProviders/ExplicitInterfaceMemberCompletionProvider.CompletionSymbolDisplay.cs
  Disallow null checking discard parameters (dotnet#58952)
  Add extension method
  Escape type arguments
  Few fixes
  Update tests.
  Add Analyzers layer to CODEOWNERS
  Add formatting analyzer test for param nullchecking (dotnet#58936)
  Move reading HideAdvancedMembers option up (dotnet#58747)
  List patterns: Slice value is assumed to be never null (dotnet#57457)
  ...
@RikkiGibson RikkiGibson modified the milestones: Next, 17.2.P1 Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants