-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
IDE0025 strangely indents when a comment follows the property name #38057
Comments
This appears to be a formatting issue. We are property putting an elastic trivia at the start of the the formatting engine is so complex here, and i have no idea what portion is even responsible for handling this, or how it would need to be updated to take caer of this. @heejaechang can you shed some light here? |
Investigating a little, there appear to be two hurdles.
To demonstrate the latter point, with or without adding elastic whitespace before public class C
{
int // comment
test;
} Formats as: public class C
{
int // comment
test;
} It does not apply further indentation to The reason why an 'enter' keypress indents the way you expect, four spaces further in than the indentation level of the property itself, is that it falls through the big https://github.com/dotnet/roslyn/blob/main/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Indentation/CSharpIndentationService.Indenter.cs#L74 method, finds nothing special to do, and does the default indentation. The extra level of indentation is added at https://github.com/dotnet/roslyn/blob/main/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Formatting/BottomUpBaseIndentationFinder.cs#L149 and is determined by https://github.com/dotnet/roslyn/blob/main/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Indentation/CSharpIndentationService.Indenter.cs#L466. So an enter keypress will add an additional increment of indentation by default. When it does not add this additional increment of indentation, such as after a semicolon or closing brace of a class member, this is because it no longer falls through the big indentation service syntax switch, and instead hits lines that override the |
My proof of concept worked! It's one approach. It adds more indentation operations within single-line constructs which represent the wrapping that they should have if they were to break across multiple lines. main...jnm2:38057_wrapping_indentation_rules |
Unfortunately, following this up is a lot of work. We wouldn't want to just capture this "extra level of indentation" choice when formatting elastic trivia, we'd want to follow all the logic in https://github.com/dotnet/roslyn/blob/main/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Indentation/CSharpIndentationService.Indenter.cs which special-cases dozens of scenarios to indent properly when you press Enter. That logic itself calls into the formatter to make decisions. So the formatter can't easily call into this indentation-on-new-line logic. Longterm, perhaps all this logic could be reimplemented by adding IndentBlockOperations computed by the formatting rules, making the formatting rules the source of truth. This would position the formatting rules to be able to codify things like how to wrap long lines, which is a community request: #15406 In this approach, the logic would change form. Instead of introspecting and deciding on the fly, spans would be computed up front along with the other indentation blocks. These spans would start one character in from the start of a syntactic construct, because pressing Enter there should indent on the next line, and likewise end one character in from the end of the construct. When braces and brackets and embedded statements are introduced, they will have to mesh well. I started experimenting with this and it seems like a great direction to go in, but it isn't a small investment. In the meantime, fixing this bug probably requires the code fix to manually compute the indentation for the added |
Version Used:
3.3.0-beta3-19413-06+ac06df1bffb7b7fd0e5bf63cc91921af76b21e03
Steps to Reproduce:
Run IDE0025.
Expected Behavior:
or
or
Actual Behavior:
The text was updated successfully, but these errors were encountered: