Configurability of ktlint #2711
Replies: 9 comments 19 replies
-
I'm curious what others think about this. It seems like the configuration options
Configurable formatting linters that I've used in the past took a different approach entirely: You could specify instances of rules and give each rule different options. Rules also had some common options, such as including or excluding certain names or annotations. If we went that route, then I think a simple INI format as in Those are just some ideas that come to my mind; I'd appreciate some input. |
Beta Was this translation helpful? Give feedback.
-
I'd lean towards less complexity. Configurability leads to bikeshedding. |
Beta Was this translation helpful? Give feedback.
-
I use ktlint in both all my private and production projects. The main reason for this:
I prefer to use the standard configuration and don't like to configure too much because this will lead to discussions in the team about it. What I don't like is to reformat the whole project when updating to a new ktlint version. detekt is a good example for having good defaults and a stability of rules, so the maintenance effort is very small. |
Beta Was this translation helpful? Give feedback.
-
This is all really good feedback. Sorry I've been so quiet. I was on vacation, and then I've been scrambling to catch up at my day job. I figured there would be push-back on the idea of expanding beyond It also sounds like it's important that updates to I'm curious about the dependencies between rules. I understand why certain configuration options may affect multiple rules, but I'm wondering how and why rules depend on each other. @paul-dingemans, could you link me to where that was discussed in the past? It did give me an idea, however. First of all, I think making rules independent would help keep things maintainable. But taking that a step further, what if we split the rules out more granularly? For example, there's one annotation wrapping rule that handles annotations in all contexts. Instead, this could be split into function annotation wrapping, class annotation wrapping, property annotation wrapping, etc. This would make the existing rule disabling capability more powerful. The wrapping could also be configured differently depending on the context. Each rule would also be more focused and easier to test. Of course, we would want backwards-compatibility, so all the annotation wrapping rules could be part of a rule group, so that existing configurations that disable the annotation wrapping rule would disable all the rules in that group. |
Beta Was this translation helpful? Give feedback.
-
Besides the kotlin version compatibility issue, there's also the IDE plugin compatibility. I.e. if the IDE flags different issues than running ktlint in CI via a gradle plugin, then it's super confusing for developers to clean everything in their IDE, then get flagged in CI for ktlint errors. |
Beta Was this translation helpful? Give feedback.
-
Hmm... I was looking at https://github.com/nbadal/ktlint-intellij-plugin and it looks like it's a compile time selection? |
Beta Was this translation helpful? Give feedback.
-
ahh...I didn't see that info on the home page for the plugin. I misread this bit: |
Beta Was this translation helpful? Give feedback.
-
In response to this and the threads proceeding this. First of all, @paul-dingemans thank you for everything you've done for ktlint. This tool has provided me with the ability to format all my code in a way that meets most of my needs. You've done a huge service for the community. I also appreciate how reliable you are with fixes and implementing most suggestions, and also how you explain your reasoning behind when you reject an issue or PR. I find the feedback here valid but do feel it got too harsh at times. We all owe volunteer maintainers gratitude and respect for their efforts. Clearly @paul-dingemans decision's weren't made lightly. I know none of that is very constructive. I couldn't read everything that was said but I did see a lot of negative emotions flaring up and just thought we should all remember to be grateful for eachother. That being said, I do agree it was probably a mistake to ignore all those upvotes on that issue that sparked all this. I have no stake in that issue and didn't read everything in depth so I don't mean to re-ignite anything, just saying I can see both sides. But anyway, speaking of configurability and speaking of rejecting a PR 😂, I know that this is also risking being off topic but I have to bring up #2537. You could say its more about integratability than configurability, but I think there is some overlap in spirit. Configuration on the level of integrations instead of rule logic, you could say. I do think I understand your reasoning for closing it, but given this conversation came up I thought I'd check in. Do you still feel there is no way forward there in terms of removing all filesystem IO without compromising filepath-based rule logic? If that sort of change matches the spirit of this goal of increased configurability such that we are willing to readdress that issue, I'd be open to working on a new PR towards it (when I find the time). I would take advice first on what I should do to improve the likelihood such a PR would be approved. Original issue: #2532 |
Beta Was this translation helpful? Give feedback.
-
And here is my much more on topic comment. I really like IntelliJ's when {
transform != null -> append(transform(element))
element is CharSequence? -> append(element)
element is Char -> append(element)
else -> append(element.toString())
} I brought this up in #2544 and #2542. Both were rejected because horizontal alignment is against official kotlin style guides. While I understand that, I don't care. In my opinion the aligned branches in the when statement above is beautiful and readable and I need it. So on the topic of configurability, I would like to configure ktlint to allow this. As a start, I need the I will probably eventually get around to this for myself anyway by copying and pasting the |
Beta Was this translation helpful? Give feedback.
-
Since I became maintainer of ktlint, I have tried to limit the configurability of ktlint to reduce complexity as much as possible. In parts of the community there is a strong desire to have more configuration options.
Let discuss about configurabilty here! What would be the correct level of configurability? How can we keep it maintainable in the long run? How should the configuration be stored in a project? How do we decide on the choice of the default value of new configuration settings?
Beta Was this translation helpful? Give feedback.
All reactions