Skip to content

Support trailing commas and comments in .slnf files#51962

Merged
marcpopMSFT merged 3 commits intomainfrom
copilot/revise-create-from-filtered-solution-file
Feb 3, 2026
Merged

Support trailing commas and comments in .slnf files#51962
marcpopMSFT merged 3 commits intomainfrom
copilot/revise-create-from-filtered-solution-file

Conversation

Copy link
Contributor

Copilot AI commented Dec 2, 2025 •
edited
Loading

  • Replace net8.0 with $(CurrentTargetFramework) in App.csproj and Lib.csproj
  • Update .github/copilot-instructions.md to document using $(CurrentTargetFramework) for test projects
  • Convert App.sln to App.slnx format
  • Update both .slnf files to reference App.slnx instead of App.sln
  • Verify all tests still pass (4/4 tests passed)
Original prompt

This section details on the original issue you should resolve

<issue_title>Revise CreateFromFilteredSolutionFile implementation for trailing comma (and allowing comments)</issue_title>
<issue_description>The current implementation of this method doesn't seem to allow trailing commas (and comments).

MSBuild supported trailing commas (and comments) of slnf files in dotnet/msbuild#6346

sdk/src/Cli/dotnet/SlnFileFactory.cs

Line 92 in f578225

JsonElement root = JsonDocument.Parse(File.ReadAllText(filteredSolutionPath)).RootElement;

@baronfel @rainersigwald Another discrepancy due to not having a unified implementation/helper for how to parse solutions and solution filters. Not specific to test command this time :) </issue_description>

Comments on the Issue (you are @copilot in this section)

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Revise CreateFromFilteredSolutionFile for trailing commas and comments Support trailing commas and comments in .slnf files Dec 2, 2025
Copilot AI requested a review from marcpopMSFT December 2, 2025 01:28
@marcpopMSFT
Copy link
Member

marcpopMSFT commented Dec 5, 2025

@copilot please replace the two new net8.0 target frameworks you added with $(CurrentTargetFramework). Please update the copilot instructions to let future copilots know to always use $(CurrentTargetFramework) by default when creating new test projects.

@marcpopMSFT marcpopMSFT marked this pull request as ready for review December 5, 2025 21:36
Copilot AI review requested due to automatic review settings December 5, 2025 21:36
@marcpopMSFT
Copy link
Member

marcpopMSFT commented Dec 5, 2025

Tested in codespaces with a slnf that had trailing commas and comments.

Copy link
Contributor Author

Copilot AI commented Dec 5, 2025

@copilot please replace the two new net8.0 target frameworks you added with $(CurrentTargetFramework). Please update the copilot instructions to let future copilots know to always use `$(CurrentTarg...

Done in 5fac8cb. Replaced both instances of net8.0 with $(CurrentTargetFramework) and added the guidance to the copilot instructions file.

Copilot AI reviewed Dec 5, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for trailing commas and JSON comments in solution filter (.slnf) files, aligning the SDK's behavior with MSBuild's support added in dotnet/msbuild#6346. The change modifies the JSON parsing in SlnFileFactory.CreateFromFilteredSolutionFile to allow these relaxed JSON features, improving developer experience when editing .slnf files.

Key Changes

  • Modified SlnFileFactory.cs to pass JsonDocumentOptions with AllowTrailingCommas = true and CommentHandling = JsonCommentHandling.Skip when parsing .slnf files
  • Added comprehensive test coverage with two new test cases validating both trailing commas and comments work correctly
  • Created TestAppWithTrailingCommaSlnf test asset with two .slnf files demonstrating both features

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Cli/dotnet/SlnFileFactory.cs Configured JsonDocumentOptions to allow trailing commas and skip comments when parsing .slnf files
test/dotnet.Tests/CommandTests/Solution/List/GivenDotnetSlnList.cs Added two parameterized tests validating both "sln" and "solution" commands work with trailing commas and comments in .slnf files
test/TestAssets/TestProjects/TestAppWithTrailingCommaSlnf/AppWithTrailingComma.slnf Test asset demonstrating .slnf with trailing comma and comments
test/TestAssets/TestProjects/TestAppWithTrailingCommaSlnf/AppWithComments.slnf Test asset demonstrating .slnf with multi-line and single-line comments
test/TestAssets/TestProjects/TestAppWithTrailingCommaSlnf/App.sln Base solution file referenced by the test .slnf files
test/TestAssets/TestProjects/TestAppWithTrailingCommaSlnf/App/App.csproj App project referenced in the test solution
test/TestAssets/TestProjects/TestAppWithTrailingCommaSlnf/App/Program.cs Simple console app for test asset
test/TestAssets/TestProjects/TestAppWithTrailingCommaSlnf/Lib/Lib.csproj Library project referenced in the test solution
test/TestAssets/TestProjects/TestAppWithTrailingCommaSlnf/Lib/LibClass.cs Simple library class for test asset

Copilot AI requested a review from marcpopMSFT December 5, 2025 21:47
@build-analysis build-analysis bot mentioned this pull request Dec 6, 2025
Open
@marcpopMSFT marcpopMSFT force-pushed the copilot/revise-create-from-filtered-solution-file branch from 5fac8cb to 7c0cb86 Compare December 13, 2025 00:11
@marcpopMSFT marcpopMSFT requested a review from MiYanni December 13, 2025 00:12
@build-analysis build-analysis bot mentioned this pull request Dec 13, 2025
Open
3 tasks
MiYanni
MiYanni approved these changes Dec 19, 2025
View reviewed changes
test/TestAssets/TestProjects/TestAppWithTrailingCommaSlnf/App.sln Outdated
Copy link
Member

@MiYanni MiYanni Dec 19, 2025

Choose a reason for hiding this comment

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

Should we be using .slnx for new test projects or does .slnf not support .slnx?

Copy link
Member

@baronfel baronfel Dec 19, 2025

Choose a reason for hiding this comment

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

Slnf are supported for slnx - we could make a point of using slnx on new tests if we wanted to. I don't feel super strongly about it because the formats are the same underlying data model. If there were a larger divergence in behavior then I'd be pushing for more slnx test examples.

Copy link
Member

@MiYanni MiYanni Dec 19, 2025

Choose a reason for hiding this comment

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

I think the benefit for .slnx is "diff-ability". So if you wanted to update the test, using .slnx would make it easier. That's the only reason I suggested it.

Copy link
Member

@baronfel baronfel Dec 19, 2025

Choose a reason for hiding this comment

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

Oh yeah, that's a great point! It's just a better format overall.

@marcpopMSFT marcpopMSFT force-pushed the copilot/revise-create-from-filtered-solution-file branch from 7c0cb86 to 690579b Compare January 12, 2026 18:21
This was referenced Jan 12, 2026
Open
Open
Open
Open
@marcpopMSFT marcpopMSFT force-pushed the copilot/revise-create-from-filtered-solution-file branch from 690579b to a98e6c3 Compare January 26, 2026 21:59
@build-analysis build-analysis bot mentioned this pull request Jan 27, 2026
Open
3 tasks
@build-analysis build-analysis bot mentioned this pull request Jan 27, 2026
Open
3 tasks
Copilot AI and others added 2 commits January 30, 2026 15:19
@marcpopMSFT
Add support for trailing commas and comments in .slnf files
b7d8159 
Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
@marcpopMSFT
Replace net8.0 with $(CurrentTargetFramework) in test projects and up…
600c74b 
…date copilot instructions

Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
@marcpopMSFT marcpopMSFT force-pushed the copilot/revise-create-from-filtered-solution-file branch from a98e6c3 to 600c74b Compare January 30, 2026 23:19
@marcpopMSFT
Copy link
Member

marcpopMSFT commented Feb 3, 2026

@copilot can you change the new test to use the new slnx file format and change the two slnf files to target the App.slnx?

@marcpopMSFT
Convert test to use .slnx format instead of .sln
d551959 
Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 3, 2026

@copilot can you change the new test to use the new slnx file format and change the two slnf files to target the App.slnx?

Done in d551959. Converted App.sln to App.slnx and updated both .slnf files to reference App.slnx.

Copilot AI requested a review from marcpopMSFT February 3, 2026 00:15
@marcpopMSFT marcpopMSFT merged commit 5edb66e into main Feb 3, 2026
23 checks passed
@marcpopMSFT marcpopMSFT deleted the copilot/revise-create-from-filtered-solution-file branch February 3, 2026 19:40
@MichaelSimons MichaelSimons mentioned this pull request Feb 3, 2026
Merged
@dotnet-maestro dotnet-maestro bot mentioned this pull request Feb 4, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@baronfel baronfel baronfel left review comments

@MiYanni MiYanni MiYanni approved these changes

Copilot code review Copilot Copilot left review comments

@marcpopMSFT marcpopMSFT Awaiting requested review from marcpopMSFT

Assignees

@marcpopMSFT marcpopMSFT

Copilot code review Copilot

Labels

Area-NetSDK

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Revise CreateFromFilteredSolutionFile implementation for trailing comma (and allowing comments)

5 participants

@marcpopMSFT @baronfel @MiYanni