Skip to content

Conversation

@xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Sep 29, 2025

The only valid option values are ignore and false. The default option value is false, so the invalid option value do_not_ignore was implicitly false.

https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/csharp-formatting-options#csharp_space_around_declaration_statements

image

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Sep 29, 2025
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 29, 2025
@xtqqczze
Copy link
Contributor Author

Build Analysis is green.

@akoeplinger
Copy link
Member

the original PR which added the option (dotnet/roslyn#15020) explicitly had ignore/do_not_ignore as options and there's also a test in roslyn that verifies this option parses the same as false. Maybe we should just update the docs?

I'm mainly asking because this value is set in almost all .editorconfig files in the dotnet github org

@akoeplinger akoeplinger added area-Infrastructure and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 3, 2025
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

@xtqqczze
Copy link
Contributor Author

xtqqczze commented Nov 3, 2025

the original PR which added the option (dotnet/roslyn#15020) explicitly had ignore/do_not_ignore as options and there's also a test in roslyn that verifies this option parses the same as false. Maybe we should just update the docs?

It doesn't look that PR actually implemented a do_not_ignore option. In any case, the current implementation only uses ignore and false:

https://github.com/dotnet/roslyn/blob/e628ec67ac5d6cf4ab52650cee1eee45a7738ff6/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Formatting/CSharpFormattingOptions2.cs#L118-L130

@xtqqczze
Copy link
Contributor Author

xtqqczze commented Nov 3, 2025

@akoeplinger I've updated the PR description to show this is explicitly flagged as an error in JetBrains Rider.

@akoeplinger
Copy link
Member

What Rider does is not relevant here IMO, they've probably just implemented what the docs say.

https://github.com/dotnet/roslyn/blob/e628ec67ac5d6cf4ab52650cee1eee45a7738ff6/src/Workspaces/CSharpTest/Formatting/EditorConfigOptionParserTests.cs#L111-L118 shows the Roslyn test, the implementation just treats any value that is not ignore as false.

Would you mind filing a roslyn issue about this?

I don't particularly care what we set but there are over 60 hits in the dotnet org and I'm not sure it makes sense to just change runtime: https://github.com/search?q=org%3Adotnet+%22do_not_ignore%22+path%3A.editorconfig&type=code

@xtqqczze
Copy link
Contributor Author

xtqqczze commented Nov 3, 2025

Opened dotnet/roslyn#80996

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Infrastructure community-contribution Indicates that the PR has been added by a community member

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants