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

Add trailing whitespace rule #780

Merged
merged 1 commit into from Mar 10, 2018

Conversation

misaelmt
Copy link

@misaelmt misaelmt commented Mar 8, 2018

I didn't add <compliant> and <noncompliant> blocks to the KDoc because it's really easy to understand what the rule does and it's hard to see those white spaces at the end of the lines in the KDoc comments.

@misaelmt misaelmt closed this Mar 8, 2018
@misaelmt misaelmt deleted the feat/no-railing-whitespaces branch March 8, 2018 17:08
@misaelmt misaelmt restored the feat/no-railing-whitespaces branch March 8, 2018 21:32
@misaelmt misaelmt reopened this Mar 8, 2018
@misaelmt misaelmt force-pushed the feat/no-railing-whitespaces branch 5 times, most recently from 51e3918 to 082c4c2 Compare March 9, 2018 02:01
Copy link
Member

@schalkms schalkms left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

@vanniktech vanniktech left a comment

Choose a reason for hiding this comment

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

Another candidate that would benefit from auto correction.

@misaelmt
Copy link
Author

misaelmt commented Mar 9, 2018

What should I do to add this feature to auto correction? It would be nice to add it in a follow up PR.

@vanniktech
Copy link
Contributor

I proposed auto correction here but it didn't get approved. I'm just nagging on each rule that would benefit from it in hope that we'll get auto correcting.

@schalkms
Copy link
Member

schalkms commented Mar 9, 2018

As @vanniktech mentioned atm detekt does not have rules with auto correction.
The PR is fine. Nothing more to do here.

@misaelmt misaelmt force-pushed the feat/no-railing-whitespaces branch from 082c4c2 to 3a98bde Compare March 9, 2018 20:58
Debt.FIVE_MINS)

override fun visitKtFile(file: KtFile) {
val lines = file.text.splitToSequence("\n")
Copy link
Member

Choose a reason for hiding this comment

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

This should be added to the FileParsingRule.

Copy link
Member

@arturbosch arturbosch left a comment

Choose a reason for hiding this comment

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

Like the other two rules, lets make use of FileParsingRule to not split every file multiple times to a line sequence ^^

@misaelmt misaelmt force-pushed the feat/no-railing-whitespaces branch from 3a98bde to 72f9aca Compare March 10, 2018 11:43
@misaelmt
Copy link
Author

@arturbosch I addressed your comments. Please re-review at your convenience.
IMO KtFileContent.content should be renamed to KtFileContent.lines

@arturbosch arturbosch merged commit f23a648 into detekt:master Mar 10, 2018
@misaelmt misaelmt deleted the feat/no-railing-whitespaces branch March 10, 2018 12:54
@arturbosch arturbosch added this to the RC6-4 milestone Mar 15, 2018
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

5 participants