Move wrapping logic from IndentationRule to new WrappingRule #1399
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Wrapping logic belong in an own rule, so it can be disabled without disabling the indentation logic. The WrappingRule contains a simplified indentation logic. Whenever a linebreak is inserted, it determines the indentation based on the current indentation of the parent node regardless whether this node is indented correctly. It is the responsibility of the IndentationRule, which runs later than the WrappingRule to fix the indentations. Spec files used in the unit tests have been split. Some parts are moved to new spec files specific for the WrappingRule. Other parts are moved to real unit tests. And on some spec files, both the wrapping and indentation rules are run together. Those files need further refactoring. The RuleExtension now also allows the diffFileLint and diffFileFormat to run on a list of rules. Spec files which are used for linting with multiple rules must specify the rule-id in the expectations. The rule-id now optionally can also be specified in those expectation when the file is checked by only one rule. However, in case the detail property of the LintError contains a ":" then the rule-id *must* be specified as it is used as delimiter. Closes pinterest#835
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Wrapping logic belong in an own rule, so it can be disabled without disabling the indentation logic.
The WrappingRule contains a simplified indentation logic. Whenever a linebreak is inserted, it determines the indentation based on the current indentation of the parent node regardless whether this node is indented correctly. It is the responsibility of the IndentationRule, which runs later than the WrappingRule to fix the indentations.
Spec files used in the unit tests have been split. Some parts are moved to new spec files specific for the WrappingRule. Other parts are moved to real unit tests. And on some spec files, both the wrapping and indentation rules are run together. Those files need further refactoring.
The RuleExtension now also allows the diffFileLint and diffFileFormat to run on a list of rules. Spec files which are used for linting with multiple rules must specify the rule-id in the expectations. The rule-id now optionally can also be specified in those expectation when the file is checked by only one rule. However, in case the detail property of the LintError contains a ":" then the rule-id must be specified as it is used as delimiter.
Closes #835
NOTE: This change is nearly impossible to review. The change is big, complex and not always straightforward. I could not find a way to make the changes smaller. Best way to test it, is running the changed code on top of multiple projects which already have been formatted with the current version of ktlint. This should not result in changes in the code.
Checklist
CHANGELOG.mdis updated