Skip to content

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

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#4999
wilzbach wants to merge 0 commit intodlang:masterfrom
wilzbach:if_constraint_same_indent

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented Dec 26, 2016

More unification of our codebase :)

(consequence of dlang/dlang.org#1534)

While this isn't automatically enforced yet, a Dscanner plugin has already been written and is in the merge queue (in fact it was used as a basis to generate these automatic replacements) and getting the Phobos codebase to a common style can be done beforehand.

=> So I am sorry that the diff is quite large - however, the idea is that we do this once & don't have to deal with this problem in the future anymore.

@jmdavis
Copy link
Member

jmdavis commented Dec 27, 2016

Can we please stop adding random formatting rules to the style guide. IMHO, this makes the constraints harder to read and creates needless churn.

@wilzbach
Copy link
Contributor Author

Can we please stop adding random formatting rules to the style guide.

Reason: #4964 (comment)

@jmdavis
Copy link
Member

jmdavis commented Dec 27, 2016

Reason: #4964 (comment)

i.e. Andrei suddenly decided that he didn't like how some piece of code looked and wanted the style guide updated to be even more restrictive, and IMHO, it's making things worse. As this PR shows, there's a lot of code in Phobos which doesn't follow that indentation, and it wasn't a problem before. I think that it should be perfectly fine to have some of the code indenting the template constraints and some not. The code was all following the style guide before, and it worked just fine. But Andrei just decided that he wanted to change the style guide - again - and poof a bunch of code is getting changed without adding any value (and making it uglier in the process IMHO).

@dukc
Copy link
Contributor

dukc commented Dec 28, 2016

Same thoughts vith Davis. Not everything needs, nor should, be standartized. Granted, readability comes first, writablity second. But that does not mean one should sacrifice all writablity for readability.

Note, I am not criticizing the work done here, I am criticizing #4964 comment.

@JackStouffer
Copy link
Contributor

Is there a way in which only the patch is scanned? So that new code conforms and the old stuff doesn't necessarily have to be touched.

@jmdavis @andralex Would this be a good compromise?

@andralex
Copy link
Member

As this PR shows, there's a lot of code in Phobos which doesn't follow that indentation, and it wasn't a problem before.

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.

@andralex
Copy link
Member

@JackStouffer one way or another we get to the same point. I'd be more worried if we had no mechanical enforcement (in which case we'd look at doing this once in a while).

if (isInputRange!Range1 &&
isInputRange!Range2 &&
!isInfinite!Range1 &&
!isInfinite!Range2)
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel these following lines should be reduced to be indented once now as well?

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.

6 participants