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

Fix multiple corrections on same offense #72

Merged
merged 2 commits into from
Feb 21, 2018

Conversation

Mangoov
Copy link

@Mangoov Mangoov commented Feb 18, 2018

#69 introduced stored offenses in an instance variable @offenses.

However this makes it so the break condition of the lopp: https://github.com/Shopify/erb-lint/blob/8b7863485fd9d9cdb36f83af8cc87cb0fc50b683/lib/erb_lint/cli.rb#L102-L103

Will only break if no offenses are detected on the first pass, if an offense is detected the first time, the loop will not break because the offences are now stored in the ivar @offense of the runner.

To circumvent this, I am comparing what the previous offenses were and breaking if there are no changes at the end of the iteration.


This PR also fixes a few misleading messages on offense

@Mangoov Mangoov force-pushed the fix-multiple-correction-same-offense branch 2 times, most recently from fa7a22f to ce71ecf Compare February 18, 2018 06:57
@Mangoov Mangoov changed the title Fix multiple correction same on same offense Fix multiple corrections on same offense Feb 18, 2018
@Mangoov Mangoov force-pushed the fix-multiple-correction-same-offense branch from ce71ecf to c9cf164 Compare February 18, 2018 07:07
@Mangoov Mangoov force-pushed the fix-multiple-correction-same-offense branch from c9cf164 to c44636e Compare February 20, 2018 18:12
@@ -113,6 +115,7 @@ def run_with_corrections(filename)
end

file_content = corrector.corrected_content
break if previous_offenses.eql?(runner.offenses)

Choose a reason for hiding this comment

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

Won't the instance of this array always be equal to itself?

Copy link
Author

@Mangoov Mangoov Feb 20, 2018

Choose a reason for hiding this comment

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

You're right, I changed the strategy to instead clear the ivars at the end of the loop

Choose a reason for hiding this comment

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

🆒 since the list is append-only you could also look at the array size

@Mangoov Mangoov merged commit c739221 into master Feb 21, 2018
@Mangoov Mangoov deleted the fix-multiple-correction-same-offense branch February 21, 2018 15:42
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.

3 participants