Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix indentation rule has problems with ignoring braces #1646

Merged
merged 4 commits into from
Sep 16, 2022

Conversation

zsqw123
Copy link
Contributor

@zsqw123 zsqw123 commented Sep 11, 2022

Description

fix ktlint indentation rule has problems with ignoring braces #1644

Checklist

  • PR description added
  • tests are added
  • KtLint has been applied on source code itself and violations are fixed
  • documentation is updated
  • CHANGELOG.md is updated

@paul-dingemans
Copy link
Collaborator

Thx for your contribution. I like the idea. The way it is implemented is a bit hard to understand. So I will refactor your solution before merging.

@paul-dingemans paul-dingemans added this to the 0.48.0 milestone Sep 13, 2022
@paul-dingemans paul-dingemans self-assigned this Sep 13, 2022
@zsqw123
Copy link
Contributor Author

zsqw123 commented Sep 14, 2022

In fact, my idea is simple. I just change the logic of suppress from never visit to never emit. And I tested it locally, and it passed the unit test.

Rename PreparedCode to RuleExecutionContext as changing the suppressionLocator in a Context feels less strange than in PreparedCode which sounds more immutable.

Extract SuppressHandler so that the logic that alters the autoCorrect and emit variables is in one place.
@paul-dingemans
Copy link
Collaborator

In fact, my idea is simple. I just change the logic of suppress from never visit to never emit. And I tested it locally, and it passed the unit test.

Yes the idea is solid and your solution worked fine! I wanted a different implementation so that the logic for determining the new autoCorrect and emit values in a single place (now in the SuppressHandler).

@zsqw123
Copy link
Contributor Author

zsqw123 commented Sep 15, 2022

#1644 is different from #1645. This pr fixes #1644 only, #1645 may be because of startOffset rather than this, and #1645 has not added a unit test.

@paul-dingemans
Copy link
Collaborator

#1644 is different from #1645. This pr fixes #1644 only, #1645 may be because of startOffset rather than this, and #1645 has not added a unit test.

Tnx for pointing that out. I didn't check that carefully enough.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants