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

Add analyze open documents only #2346

Merged
merged 4 commits into from
Mar 3, 2022

Conversation

neoGeneva
Copy link
Contributor

Hey, this is a pull requestion to add an AnalyzeOpenDocumentsOnly option to RoslynExtensionsOptions, which CSharpDiagnosticWorkerWithAnalyzer uses to decided if it runs full roslyn anaylizers or just the base diagnostics provided by syntaxTree.GetDiagnostics().

The main benefit is the speed of the initial analysis of all files in the solution. For large solutions this is pretty handy, for example the roslyn solution takes 18 minutes for that first pass, vs. 1 minute with AnalyzeOpenDocumentsOnly set to true.

@dnfadmin
Copy link

dnfadmin commented Feb 17, 2022

CLA assistant check
All CLA requirements met.

Copy link
Member

@filipw filipw left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! - any thoughts @333fred @JoeRobich ?

@filipw filipw enabled auto-merge March 3, 2022 20:08
@filipw filipw merged commit f2fbbd4 into OmniSharp:master Mar 3, 2022
@neoGeneva
Copy link
Contributor Author

Wooo! Thanks so much for this folks, this is my first real contribution to a public repo, feels good to help out ❤️

@333fred
Copy link
Contributor

333fred commented Mar 3, 2022

@neoGeneva, would you want to submit a PR to the vscode to add the option? You'll need to:

My suggestion would be off (false) by default for this setting.

@neoGeneva
Copy link
Contributor Author

Sweet, thanks @333fred, I've got most of it ready to go, but it got mixed up with some other stuff I was looking at, just splitting it out into its own branch so I can do a pull request a little later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants