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

Fix solution parsing (again!) #918

Merged
merged 4 commits into from
Jul 19, 2017

Conversation

DustinCampbell
Copy link
Contributor

It turns out that my recent change to use the MSBuild solution parsing API (for a second time) has broken Unity solutions. In this particular case, the MSBuild API throws if a solution file contains multiple projects with the same name, which is how Unity solutions are generated. Thanks to @Leopotam for reporting this issue!

The MSBuild API is much more restrictive about the format of a solution file than Visual Studio. I suppose that makes sense given that MSBuild can fail fast if a solution file is invalid. However, an IDE must be more resilient against faults. So, I've written a brand new solution parser from scratch. It is heavily derived from the parser in MSBuild.

Fixes dotnet/vscode-csharp#1645

@DustinCampbell
Copy link
Contributor Author

Note: I'm expecting AppVeyor to fail initially due to the outage their experiencing around NuGet (http://status.appveyor.com/incidents/m2vdvw39kdk8). I'm currently tracking the incident there and will re-kick the CI once it's resolved.

Also, I'm betting the OSX build will fail as it seems to time out a lot during US business hours. I'll kick it later if it does time out.

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!

@david-driscoll david-driscoll merged commit 91b884f into OmniSharp:master Jul 19, 2017
@DustinCampbell DustinCampbell deleted the new-solution-parser branch August 30, 2017 19:13
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.

3 participants