Skip to content

DStyle: Constraints on declarations should have the same indentation level#5139

Closed
wilzbach wants to merge 0 commit intodlang:masterfrom
wilzbach:if_constraint_same_indent
Closed

DStyle: Constraints on declarations should have the same indentation level#5139
wilzbach wants to merge 0 commit intodlang:masterfrom
wilzbach:if_constraint_same_indent

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented Feb 16, 2017

I accidentally closed #4999 - the decision from this PR was:

It is a problem. Phobos is a messy mix of styles, which makes it gratuitously difficult to work with. Let's not argue this please - it's subjective so a decision needs be made. @wilzbach please rebase and let's pull. Thanks!
Expect future improvements to the style guide followed by automated enforcements.

I updated the replacement tool that looks at all Dscanner warnings and tries to fix them, so that a couple of previously unhandled edge case are handled. Moreover it now also updates the indentation of the following lines of the if constraint correctly (+4 of the if indentation level).

Steps to reproduce

1) Git clone the specific branch of the dscanner

git clone https://github.com/wilzbach/dscanner ../dscanner
git -C ../dscanner if_constraints_same_indent

2) Modify dscanner to emit the expected indent size

--- a/src/analysis/if_constraints_indent.d
+++ b/src/analysis/if_constraints_indent.d
@@ -101,8 +101,10 @@ private:
                                                        .until!(x => !x.isWhite)
                                                        .walkLength;
 
+         import std.conv : to;
                if (tokenIndent != lineIndent)
-                   addErrorMessage(constraint.expression.line, constraint.expression.column, KEY, MESSAGE);
+                 addErrorMessage(constraint.expression.line, constraint.expression.column, KEY,
+                         MESSAGE ~ ":" ~ tokenIndent.to!string);
        }
 }

3) Ignore other checks

Add the following to the .dscanner.ini:

allman_braces_check = "disabled"
trailing_whitespace_check = "disabled"
consecutive_empty_lines = "disabled"

4) Run dscanner and save all warnings

make -f posix.mak style > list

5) Let the replacement tool fix all warnings

rdmd --compiler=../dmd/src/dmd ../replacements/replacement.

&& is(typeof(InputRange.init.front = ForwardRange.init.front)))
if (isInputRange!InputRange
&& (isForwardRange!ForwardRange
|| (isInputRange!ForwardRange && isInfinite!ForwardRange))
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this line still be indented? The || is part of a sub condition.

(isForwardRange!ForwardRange || (isInputRange!ForwardRange && isInfinite!ForwardRange))

Putting it at the same level makes it confusing, with the && and || aligned.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @sprinkle131313 but let's leave that to the next pass.

@andralex
Copy link
Member

Auto-merge toggled on

@andralex
Copy link
Member

@wilzbach all good, rebase and babysit the autotester

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants