Skip to content
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

Improved indentation handling of IndentationHelper.GetIndentationSteps #1751

Merged
merged 1 commit into from
Nov 11, 2015

Conversation

vweijsters
Copy link
Contributor

Fixes #1036
Fixes #1037

For #1037 I've implemented a fourth option: rounding. For indentation size 4 this means: 0-1 no indent, 2-3 single indent. This is my personal preference, as it is forgiving when there is one space to many / little. I'll be happy to adapt it if wanted.

@codecov-io
Copy link

Current coverage is 79.44%

Merging #1751 into master will not affect coverage as of e3ab60a

@@            master   #1751   diff @@
======================================
  Files          546     548     +2
  Stmts        32764   32797    +33
  Branches      9078    9083     +5
  Methods                          
======================================
+ Hit          26029   26055    +26
- Partial       5273    5278     +5
- Missed        1462    1464     +2

Review entire Coverage Diff as of e3ab60a

Powered by Codecov. Updated on successful CI builds.

@sharwell
Copy link
Member

This seems fine.

I do wonder if we should instead simply use the previous indentation string unaltered, rather than trying to rewrite it using the currently configured indentation characters. But that is a separate issue.

sharwell added a commit that referenced this pull request Nov 11, 2015
Improved indentation handling of IndentationHelper.GetIndentationSteps
@sharwell sharwell merged commit 564fcc5 into DotNetAnalyzers:master Nov 11, 2015
@sharwell sharwell added this to the 1.0.0 Beta 16 milestone Nov 11, 2015
@sharwell sharwell self-assigned this Nov 11, 2015
@vweijsters vweijsters deleted the fix-indentationhelper branch November 11, 2015 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants