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 #132 by respecting indent_size from .editorconfig #133

Merged
merged 4 commits into from
Mar 2, 2020
Merged

Fix #132 by respecting indent_size from .editorconfig #133

merged 4 commits into from
Mar 2, 2020

Conversation

mateuszkwiecinski
Copy link
Contributor

Fixes #132

The reson why properties set in .editorconfig weren't respected is that we were always passing them in ktlint's userData which are higher in hierarchy than those set in .editorconfig.

In addition I removed support for continuation_indent_size as it is no longer recognized by ktlint pinterest/ktlint#171 (I could mark it as deprecated though 🤔WDYT?)

Remove unused `continuationIndentSize`
Copy link
Owner

@jeremymailen jeremymailen left a comment

Choose a reason for hiding this comment

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

Thank you once again!

My best guess is the upgrade from ktlint 0.34.0 to 0.34.2 broke this behavior? I have a vague memory that they used to do the precedence the other way around and they corrected it.
https://github.com/pinterest/ktlint/pull/534/files#diff-99e7e840295caf6ea84c484c30d73158R140

@@ -16,9 +14,7 @@ open class KotlinterExtension {
/** Don't fail build on lint issues */
var ignoreFailures = DEFAULT_IGNORE_FAILURES

var indentSize = DEFAULT_INDENT_SIZE

var continuationIndentSize = DEFAULT_CONTINUATION_INDENT_SIZE
Copy link
Owner

Choose a reason for hiding this comment

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

Good catch -- probably the right thing is remove it entirely in 3.0 -- leave the parameter so that nobody's gradle files blow up on upgrade, but print a WARN level output saying the setting is deprecated and does nothing.

Copy link
Owner

Choose a reason for hiding this comment

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

If you wish to remove it from the readme, you can or I'll make those updates when I push the version bump.

Copy link
Contributor Author

@mateuszkwiecinski mateuszkwiecinski Feb 23, 2020

Choose a reason for hiding this comment

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

Done ✅
Added deprecation messages, both for tasks and for the extension. Removed all continuationIndentSize references from Readme.

}

@Test
fun `lintTask uses default indentation if editorconfig absent`() {
Copy link
Owner

Choose a reason for hiding this comment

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

oh good, so that's ktlint default when there isn't userData or an .editorconfig.

Copy link
Owner

@jeremymailen jeremymailen left a comment

Choose a reason for hiding this comment

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

Brilliant!

@jeremymailen jeremymailen merged commit edd9d98 into jeremymailen:master Mar 2, 2020
@mateuszkwiecinski mateuszkwiecinski deleted the feature/respect_indent_size_set_from_editorconfig branch March 2, 2020 08:04
# 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.

Configuring indent_size from .editorconfig broken
2 participants