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

Register missing ktlint_disabled_rules in editorConfigProperties #1671

Conversation

tcrawford-figure
Copy link
Contributor

@tcrawford-figure tcrawford-figure commented Oct 7, 2022

Description

editorConfigProperties seemed to be missing the newly created ktlintDisabledRulesProperty. For any downstream plugins that rely on this value to know what properties are valid for ktlint, they may filter out anything but these properties.

Ultimately, this could leads to errors like the following if that's the only editor config override provided:

Can not create an EditorConfigOverride without properties. Use 'emptyEditorConfigOverride' instead.

Or if it's not, I can safely assume that the disabled rules will be ignored if disabled_rules aren't provided as well.

Currently, disabled_rules still works but it spams a lot of messages stating that it is disabled. I find this to be an unpleasant developer experience and also leads to confusion for a developer who is simply switching to ktlint_disabled_rules from disabled_rules only to find that their disabled rules are no longer honored.

I did not find an issue reported regarding this issue, but please feel free to link any issues if I missed something.

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

In case of adding a new rule:

@tcrawford-figure tcrawford-figure marked this pull request as ready for review October 7, 2022 14:54
@tcrawford-figure tcrawford-figure changed the title Register ktlint_disabled_rules in editorConfigProperties Register missing ktlint_disabled_rules in editorConfigProperties Oct 7, 2022
@paul-dingemans paul-dingemans added this to the 0.48.0 milestone Oct 7, 2022
@paul-dingemans
Copy link
Collaborator

Tnx for fixing this bug. The property indeed needs to be defined.

@shubham96301
Copy link

Can someone please create a release for this fix?

@paul-dingemans
Copy link
Collaborator

The next release is being finalized. It will be released in a couple of weeks.

@shubham96301
Copy link

still waiting for the release

@baseilos
Copy link

baseilos commented Dec 7, 2022

@paul-dingemans is there a timeline when this will be released?

@paul-dingemans
Copy link
Collaborator

A couple of days, if all goes well.

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

Successfully merging this pull request may close these issues.

4 participants