Skip to content

Permit comments and trailing commas in solution filter files#6346

Merged
benvillalobos merged 2 commits intodotnet:mainfrom
Forgind:trailing-comma-slnf
Apr 15, 2021
Merged

Permit comments and trailing commas in solution filter files#6346
benvillalobos merged 2 commits intodotnet:mainfrom
Forgind:trailing-comma-slnf

Conversation

@Forgind
Copy link
Contributor

@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.

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