Optionally allow to split class attribute values into multiple lines

[thet]

• Multiline values for class attributes.
  Split multiple class attribute values into multiple lines. Keep it
  single-lined, if there is only one value. And don't split Chameleon
  expressions.

• Add a switch for multiline class attribute values.
  Per default class attributes are not split into multiple lines. That's what
  most tools do and most people would expect. To split class attributes into
  multiple lines use the --split-class option on the CLI.

[coveralls]

Coverage is 97.204% — thet/class-multiline into master. No base build found for master.

[ale-rt]

This will for the first time introduce a flag that controls how zpretty formats the file.

I do not think we should add it.

Either we go for always having class names split one per line or we never do that.

For me it is fine to have classes one per line, but I think this should be discussed with the community, also because it will cause a lot of broken dependabot PRs.

If we go for such a bold move, it might also be interesting to change that only if the line length is longer than a given amount of characters (that would even make the formatter fully compliant with the ZCML style guide, see #12).

But before deciding something on this I would like to hear more opinions.

[gforcada]

My 2 cents here:

• yes, let's go all in or not, otherwise if it is behind a flag, you will need to propagate that flag everywhere (local tooling, CI, etc.)
• as for the feature itself: for long lists it does make sense to me, specially if you have chameleon expressions
• keeping it in one line if it fits within 80 characters, or then fully split it when it goes over the threshold sounds good (i.e. black behavior)

My main expectation from using a linter is to easy my life, and when it comes to commits means making diffs as small as possible, zpretty could sort the classes as to easily spot which ones have been added/renamed/removed.

That together with the multiline, could be used to group within the same line, the classes that have the same prefix (again, if they don't go over the 80 characters threshold).

So this PR looks like a good step in the right direction to me if we apply black heuristics to split lines and we add later the two wishes from me: sorting and grouping 🎉

[ale-rt]

My 2 cents here:

• yes, let's go all in or not, otherwise if it is behind a flag, you will need to propagate that flag everywhere (local tooling, CI, etc.)

I also agree that we should either go all in or not 👍

• as for the feature itself: for long lists it does make sense to me, specially if you have chameleon expressions
• keeping it in one line if it fits within 80 characters, or then fully split it when it goes over the threshold sounds good (i.e. black behavior)

Yeah, that would be nice also to solve #12 as already mentioned

My main expectation from using a linter is to easy my life, and when it comes to commits means making diffs as small as possible, zpretty could sort the classes as to easily spot which ones have been added/renamed/removed.

I think sorting/grouping might lead to undesired effects, see:

• https://stackoverflow.com/questions/15670631/does-the-order-of-classes-listed-on-an-item-affect-the-css

That together with the multiline, could be used to group within the same line, the classes that have the same prefix (again, if they don't go over the 80 characters threshold).

So this PR looks like a good step in the right direction to me if we apply black heuristics to split lines and we add later the two wishes from me: sorting and grouping 🎉

It might be interesting to split the classes by new line so that we can have something like:

<a class="foo
bar baz">
...

formatted as:

<a class="
           foo
           bar baz
         ">
...

Which would be very similar to what happens with a tal:define attribute.

Let me know, I am very open to include a similar feature.

[gforcada]

That looks better indeed 👍🏾

As for sorting/grouping, at work we are using BEM methodology for CSS, so our classes look like:

block__element block__element--modifier potentially__another potentially__another--with-modifier

So keeping the block__ classes in a single line with the ones without --modifier first makes sense and should not have side effects.

But as not everyone is using that I'm fine having to either add it ourselves on a fork of it, or giving up on the idea 😅

Could I ask for a function/method that allows a potential patch be made easy by isolating the parsing of the classes before/after the split? Then we could implement our logic there without having to force it upstream 😄

[ale-rt]

Me and Hannes are going to mee at the Buschenschanksprint and could work on that.

You are welcome to provide and example of the code should be, either in a comment or by providing failing tests, or by adding some classes here: https://github.com/collective/zpretty/blob/master/zpretty/tests/original/sample_pt.pt