-
-
Notifications
You must be signed in to change notification settings - Fork 681
max-attributes-per-line defaults to "singleline: 1", should be 3 according to docs #300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
On the other hand, the Vue Style Guide is quite clear that multiple attributes should go on newlines. So the current default behavior seems correct in this regard, only the docs need updating. I personally would like to see 3 allowed attributes in the single line case, but I would succumb to the official vue style guide in this regard. |
Hey @tiltec, thanks for posting this. Now this topic will have it's proper attention :) What do you think about this @chrisvfritz? You probably missed my proposition in bunch of other messages in the original issue :D If we're going to change anything here, it has to start with the StyleGuide, so we can align with our source of truth :) |
Since the number of max attributes really depends on the length of those attributes, I think one per line is the only bikeshed-proof choice - unless we include another option to specify max columns before wrapping. On a related note: when Prettier supports HTML, it'll wrap according to max columns. This is in the works, so my preference would probably be to wait for Prettier to take care of it for us, then release a |
History:
So I think it's a document issue. As a side note, I think that we can use prettier from this plugin as similar to https://github.com/prettier/eslint-plugin-prettier if it's pretty (though I have not used prettier). |
Yeah, I agree it's a documentation issue - sorry if I didn't make that clear. 😅 I'm not sure about using Prettier within this plugin though. Many in the Vue community make ESLint part of their Webpack build process, immediately failing if there's an error. For these users, immediately halting a build on a formatting issue would probably be very frustrating. I think the strategies of eslint-config-prettier and stylelint-config-prettier are more flexible, so that no matter how Prettier is included in a project, we have a patch ruleset that will avoid any conflicts, deferring to Prettier wherever there's overlap. |
Okay, I agree @chrisvfritz. I just wanted to point that it would be good to consider more attributes in one line as it's kind of a common scenario, and I'm also used to change default setting of this rule in each project I'm working on to accept max 3 attrs in one line :) I'll update docs for now, so they're aligned with the style guide and we can observe if more people will also expect different defaults, if yes - we can iterate the style guide then. WDYT? |
I'm not opposed to changing the default to more than 1 attribute per line, but we'd need to introduce a max columns (i.e. line length) before going multiline - and once we go multiline, I'd want to again require only 1 attribute per line. The reason is that without a max columns constraint, some attributes will be too long for some people, which means the rule will have failed from preventing us from bikeshedding. 😂 Does that make sense? |
I am sorry but I think 1 is to strict default. It make my templates stretched in vertical dimension and I need to scroll up and down any time. IMHO 3 make much more sense. |
BTW, example in vue.js official guide doesn't fit this rule. |
I agree with @rzaharenkov as well. Having 2-3 attributes is still considered common and also short so it doesn't have to be strict to be in new line. For example of these cases:
Our developers would like to follow the full VueJS recommended standards too, but this rules seems too strict and our HTML template doesn't look really well after most of these elements needs to be in multi-line. I understand that we can just override the rules with custom Please reconsider, thank you! |
@rzaharenkov @chenxeed You're both welcome to configure this rule locally as long as you don't mind bikeshedding over cases where even 2 or 3 attributes is too long. 🙂 For what it's worth, I've worked with some clients that have chosen to override it and in every single case, they've eventually changed it back because they got tired of not seeing attributes when troubleshooting and arguing about which cases were appropriate for more than one attribute per line. FYI though, we'll likely disable this rule by default once Prettier supports HTML, as its line-length-based formatting will create a conflict. |
I see. Thanks for clarifying @chrisvfritz ! Having |
I personally prefer the way IntelliJ does it, where you can decide whether it wraps or chops depending on if the line is too long. If the following line were too long then it either wraps <tag attribute1="value1" attribute2="value2" attribute3="value3"
attribute4="value4" attribute5="value5" /> or it chops <tag attribute1="value1"
attribute2="value2"
attribute3="value3"
attribute4="value4"
attribute5="value5" /> but it's determined by whether the line needs to wrap or not. If that could be achieved here, it would really make my day. |
Tell us about your environment
What did you do? Please include the actual source code causing the issue.
I use the
strongly-recommended
rule that enablesmax-attributes-per-line
. It should default to 3 attributes in the singleline case, according to the docs:This should be correct:
There's also a docs inconsistency, as this code is given as incorrect in the docs:
What did you expect to happen?
Don't get a lint error.
What actually happened?
Got a lint error.
Also noticed by @michalsnik in #77 (comment)
The text was updated successfully, but these errors were encountered: