Skip to content

Allmanify Phobos and add CI check#4385

Merged
DmitryOlshansky merged 3 commits intodlang:masterfrom
wilzbach:braces_allmanify
May 31, 2016
Merged

Allmanify Phobos and add CI check#4385
DmitryOlshansky merged 3 commits intodlang:masterfrom
wilzbach:braces_allmanify

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented May 31, 2016

again with the power of regular expressions :)

# find common cases
sed -E "s/^(\s*)((if|static if|for|foreach|foreach_reverse|while|unittest|switch|else).*)\s*\{$/\1\2\n\1{/" -i **/*.d
# catch else-if
sed -E "s/^(\s*)} (else static if| if|else)(.*)\s*\{$/\1}\n\1\2\n\1{/" -i **/*.d
# remove created trailing whitespace
sed -i 's/[ \t]*$//' **/*.d

afterwards only a small manual fixup was needed.

edit:

  1. In case you aren't that familar with regular expressions - the grep just check when there is a { at the end of the line, so while (true) { } is still valid
  2. this PR doesn't check for the all-man style in functions, that will be a follow-up once Dscanner has this functionality.

@PetarKirov
Copy link
Member

PetarKirov commented May 31, 2016

Nice work! All LGTM, except for a couple template constraints in the first commit.
(We should add a rule for that to the Phobos style guide.)

@DmitryOlshansky
Copy link
Member

again with the power of regular expressions :)

Cool ;)

LGTM

wilzbach added 2 commits May 31, 2016 13:07
// find common cases
sed -E "s/^(\s*)((if|static if|for|foreach|foreach_reverse|while|unittest|switch|else|version).*)\s*\{$/\1\2\n\1{/" -i **/*.d
// catch else-if
sed -E "s/^(\s*)} (else static if| if|else if|else)(.*)\s*\{$/\1}\n\1\2\3\n\1{/" -i **/*.d
// remove created trailing whitespace
sed -i 's/[ \t]*$//' **/*.d
@wilzbach wilzbach force-pushed the braces_allmanify branch from 1867381 to a5b188f Compare May 31, 2016 11:09
@wilzbach wilzbach force-pushed the braces_allmanify branch from a5b188f to ceadc7f Compare May 31, 2016 11:11
@atilaneves
Copy link
Contributor

LGTM

@DmitryOlshansky
Copy link
Member

Auto-merge toggled on

@DmitryOlshansky DmitryOlshansky merged commit cdc8218 into dlang:master May 31, 2016
@andralex
Copy link
Member

Nice! (IIRC these aren't manly braces, Allman is an actual person.) Is this style going to be automatically enforced going forward?

@wilzbach
Copy link
Contributor Author

Is this style going to be automatically enforced going forward?

Yes this was the entire point. Currently only with non-D grep, but in the future Dscanner will support this too ;-)

@wilzbach wilzbach deleted the braces_allmanify branch May 31, 2016 14:04
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.

5 participants