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

Empty Line Separator Check, added option for managing empty lines betwee... #597

Closed

Conversation

alexkravin
Copy link
Contributor

...n class members, issue #530

Added option allowMultipleEmptyLines for managing empty lines between class members, default value is true.

Added corresponding UT and input file.

@romani
Copy link
Member

romani commented Jan 25, 2015

build is failed.

// 3 is the number of the pre-previous line because the numbering starts from zero.

need more explanation on this, previous usually mean "-1", pre-previous "-2"

@alexkravin
Copy link
Contributor Author

Corrected build.

About pre-previous line:
it was taken from existing method

just analyzing previous line from this one.

Seems to be it's the feature of FileContents class from where getLines() is calling
(getLines() is method from Check which calls return getFileContents().getLines();)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 90.62% when pulling 378c68f on alexkravin:EmptyLineSeparatorCheck#530 into 204c073 on checkstyle:master.

@romani
Copy link
Member

romani commented Jan 27, 2015

ok, but rename hasEmptyLinesBefore method to be named absolutely reflecting functionality. As "Lines" is not exact, and there is not argument that customize distance before.

@alexkravin
Copy link
Contributor Author

renamed to isPrecededByMoreThanOneEmptyLine "line" is quite exact in this context, we don't need to take care about distance as we're looking for only "> 1 empty lines", so I think it's quite obvious.

@romani
Copy link
Member

romani commented Jan 27, 2015

Build is failed.

Please send me link to line in code where you have " ... > 1" or " ... > 2" if lines are indexed from 0.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0%) to 90.64% when pulling 4ca50aa on alexkravin:EmptyLineSeparatorCheck#530 into c6019cd on checkstyle:master.

@romani
Copy link
Member

romani commented Jan 27, 2015

Please send me link to line in code where you have " ... > 1" or " ... > 2" if lines are indexed from 0.

I meant in isPrecededByMoreThanOneEmptyLine method. Method named incorrectly.

@alexkravin
Copy link
Contributor Author

generally speaking, this method checks only pre-previous line, in context of Check couple of methods are checking "more than 1 empty line before", so I renamed this method to isPrePreviousLineEmpty because it checks exactly pre-previous line.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 90.64% when pulling 8db2b14 on alexkravin:EmptyLineSeparatorCheck#530 into f46ec1b on checkstyle:master.

@romani
Copy link
Member

romani commented Jan 29, 2015

merged as FF

@romani romani closed this Jan 29, 2015
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.

None yet

3 participants