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

Permit comments and trailing commas in solution filter files #6346

Merged
merged 2 commits into from
Apr 15, 2021

Conversation

Forgind
Copy link
Member

@Forgind Forgind commented Apr 14, 2021

Fixes #6317

We previously added support for building solution filter files. With that support, we did not allow them to contain lists of projects with a trailing comma or comments. These are both permitted by VS, so this PR updates our handling of solution filter files to permit building with trailing commas or comments.

Also adds a test.

@Forgind Forgind changed the title Permit comments and trailing commas Permit comments and trailing commas in solution filter files Apr 14, 2021
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

LGTM with an added comment.

@@ -415,7 +415,7 @@ internal static string ParseSolutionFromSolutionFilter(string solutionFilterFile
{
try
{
JsonDocument text = JsonDocument.Parse(File.ReadAllText(solutionFilterFile));
JsonDocument text = JsonDocument.Parse(File.ReadAllText(solutionFilterFile), new JsonDocumentOptions() { AllowTrailingCommas = true, CommentHandling = JsonCommentHandling.Skip});
Copy link
Member

Choose a reason for hiding this comment

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

I'd go ahead and create a variable for JsonDocumentOptions so that you can comment that it should match the VS behavior.

@benvillalobos benvillalobos added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Apr 14, 2021
@benvillalobos benvillalobos merged commit 478f12f into dotnet:main Apr 15, 2021
rainersigwald added a commit to rainersigwald/msbuild that referenced this pull request May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow trailing commas in solution filters
3 participants