Formatting options refactoring - step 3#58904
Conversation
ghost
added
|
@CyrusNajmabadi PTAL |
| [Flags] | ||
| internal enum SpacePlacement | ||
| { | ||
| IgnoreAroundVariableDeclaration = 1, |
There was a problem hiding this comment.
| IgnoreAroundVariableDeclaration = 1, | |
| IgnoreAroundVariableDeclaration = 1 << 0, |
up to you :) i like the symettry.
| bool useTabs, | ||
| int tabSize, | ||
| int indentationSize, | ||
| string newLine, |
There was a problem hiding this comment.
i feel like teh above 4 often go hand in hand. any thoughts on combining to a basic formatting options struct?
| (options.GetOption(CSharpFormattingOptions2.NewLinesForBracesInAnonymousMethods) ? NewLinePlacement.BeforeOpenBraceInAnonymousMethods : 0) | | ||
| (options.GetOption(CSharpFormattingOptions2.NewLinesForBracesInLambdaExpressionBody) ? NewLinePlacement.BeforeOpenBraceInLambdaExpressionBody : 0) | | ||
| (options.GetOption(CSharpFormattingOptions2.NewLinesForBracesInControlBlocks) ? NewLinePlacement.BeforeOpenBraceInControlBlocks : 0) | | ||
| (options.GetOption(CSharpFormattingOptions2.NewLineForClausesInQuery) ? NewLinePlacement.BetweenQueryExpressionClauses : 0), |
There was a problem hiding this comment.
i don't love the difference in names here. any thought on unification?
There was a problem hiding this comment.
I named the flags based on the editorconfig names. I can follow up to rename the options as well. Tracking: #59089
| public override AbstractFormattingRule WithOptions(SyntaxFormattingOptions options) | ||
| { | ||
| var cachedOptions = new CachedOptions(options.Options); | ||
| var newOptions = options as CSharpSyntaxFormattingOptions ?? CSharpSyntaxFormattingOptions.Default; |
There was a problem hiding this comment.
yick. should we throw if we ere not given a CSharpSyntaxFormattingOptions ?
There was a problem hiding this comment.
I'm not sure how this may be used right now. I was being conservative here and preserve the behavior.
| public override AbstractFormattingRule WithOptions(SyntaxFormattingOptions options) | ||
| { | ||
| var cachedOptions = new CachedOptions(options.Options); | ||
| var newOptions = options as CSharpSyntaxFormattingOptions ?? CSharpSyntaxFormattingOptions.Default; |
| public readonly bool UseTabs; | ||
| public readonly int TabSize; | ||
| public readonly int IndentationSize; | ||
| public readonly string NewLine; |
There was a problem hiding this comment.
i'd actually love for these four to be their own struct.
There was a problem hiding this comment.
Possibly. We can do another round of refactorings/renames once finished with all options.
ryzngard
added
UX Review Not Required
5 participants
Add fields for formatting options.