Add 'aligned' option that align attributes after specific line length…

[cheerypick]

This PR is about requested soft-alignment feature of attributes alignment after and only after specific line length threshold is reached.

related to #1262

Basically, this PR adds a new option that doesn't match 'is_wrap_attributes_force' condition (that forces line break after each attribute) but indents the attributes same way as 'is_wrap_attributes_force_aligned' if 'wrapped' is set to true by wrap_line_length.

//At least, I suppose it does :)

As I said previously, I am not exactly an open source expert, so comments, guidance, and advice appreciated.
(Hope that I understood how the test matrices work correctly too)

[bitwiseman]

Thanks the PR. You understood exactly how the test matrices work.
Because of that I can say that your change doesn't quite address #1262.

In #1262, they ask for:

<div v-for="item in items" 
     class="items" 
     v-if="items.notEmpty()" 
     :class="{active: isActive} 
     :key="item.id">
    <p>{{item.name}}</p>
</div>

Whereas this PR would allow for something like (depending on the wrap line length):

<div v-for="item in items" class="items" v-if="items.notEmpty()" 
     :class="{active: isActive} :key="item.id">
    <p>{{item.name}}</p>
</div>

To do #1262, you'll need to look-ahead to see if the current element with attributes is going to wrap and then switch to force-align mode if that is the case.

In terms of quality, this is a fine PR, it just needs to be changed to implement the feature described in #1262.

[cheerypick]

Hmm, good point, I think it can be discussed. I was in first place solving my own problem and the issue in #1285, and didn't quite address what was requested in #1262.

Frankly speaking, what I was trying to recreate was Intellij's behavior: it doesn't wrap attributes before certain length but wraps and aligns them every time the line is over a threshold. If there is room for more than one attribute, so it tries to fit them, otherwise wraps. Our code style (grown from the time when the majority of devs on the project were using IntelliJ) doesn't impose that every attribute is on the new line (in this case we could just use force-align). (And personally, I think it's a bit strange behavior - like, we are fine with 3-5 attributes until line is below threshold, but suddenly if the threshold is reached, so it's one per line). I thought that all JetBrains' products behave like this, but I maybe not, and I totally understand that people may want such behavior :) I am not familiar how code formatting is configured in these IDEs and why it is different, though.

Furthermore, what was requested in #1285, demonstrates kinda the opposite behaviour - it removes existing line breaks and reindents everything according to line length and indents. I am not sure how this should work with preserve-newlines? Not sure either that my implementation will handle this correctly.

So in order to make more people happy (at least me, author of 1262 and 1285), we should maybe provide some combination of options that enables all the scenarios? Like, 'soft' indented wrapping, and force wrapping but after certain initial line length. Any architectural tips on that?

[bitwiseman]

@cheerypick
I've reopened #1285 since you've demonstrated that it is different from #1262.
I'm happy to have you address #1285 now and leave #1262 to later (or someone else). Some combination of options is exactly what will be needed. What set of setting names would you use to describe these? (Ignore the current naming if you like, I'm open to changing the values if it will make them consistent and easier to understand.)

[bitwiseman]

@cheerypick Any chance to look at this further?

[cheerypick]

Oh, @bitwiseman, sure. Had to prioritize other things, but will look at it hopefully this week.

[bitwiseman]

@cheerypick Bump. I understand if you're busy, just checking.

[RenaldasK]

Not really sure what is holding this up, but I might fork this and try to get it merged and closed, as I would love to see this feature in the next release.

[bitwiseman]

@RenaldasK
Thanks for the question. I had to go back and read again to make sure.

@cheerypick said:

So in order to make more people happy (at least me, author of 1262 and 1285), we should maybe provide some combination of options that enables all the scenarios? Like, 'soft' indented wrapping, and force wrapping but after certain initial line length. Any architectural tips on that?

I said I was fine if we wanted to have two separate PRs - this one to address #1285, another to address #1262.

If we go that route I'd still like to have the naming figured out - If #1285 behavior is aligned, what is #1262 going to be called? I want to have some plan for that. This naming structure for attribute wrapping is reaching its limits and will likely need to be designed soon anyway. Thus I asked @cheerypick what she thought the names should be.

@cheerypick also, mentioned a number of other scenarios that might or might not have side-effects in this area. I wasn't sure how to proceed.

Separately, looking at the complexity of this area, there probably needs to be another test or two, or even another matrix entry - maybe a line_wrap=40 case.

Once again, this is solid PR as it stands, I was mostly waiting on @cheerypick to make the call about what direction she wanted to go - Adding #1262 or just doing #1285.

[NEO97online]

For the naming, here's what I propose:
#1285 is soft-aligned
#1262 is aligned

#1285 is "soft" because it doesn't align every single attribute, just the lines themselves.
#1262 makes more sense to be called "aligned" imo because it's truly aligning the attributes- its the same behavior as force-aligned without being forced.

This would be really helpful for my team so I really hope we can at least get #1285 merged if that's already done, then we can work on #1262.

[vvs]

Hey @bitwiseman, @cheerypick, @michaelauderer, other folks that are interested.

Here's my take on the topic.

What we have

wrap_attributes:

• auto - enables wrapping of attributes.
• force - first attribute stays, others wrapped with "wrap_attributes_indent_size"
• force-aligned - the first attribute stays, others wrapped and aligned with the first one
• force-expand-multiline - If there are more than 1 atribute, ALL attributes are wrapped no matter what and indented with "wrap_attributes_indent_size". AND: the closing '>' of the opening tag is also on the new line. This mode is currently broken, produces wrongly indented files :(

I think that there are multiple (orthogonal) concerns here and putting them all into single value would create more confusion and weird option names explosion if we are to handle all cases.

So, let's separate wrapping, aligning and number of attributes per line, etc.

Proposal

wrap_attributes_mode:

• none - We just don't wrap attributes, period. (needed?)
• auto - Normal attribute wrapping, this should be the default, probably, as the most logical case. When the line is too big, we wrap. If all attributes can fit into a single line, they stay on the single line, no wrapping. (Default.)
• force - We forcefully wrap if there are more than 1 attribute present, otherwise auto.

When the wrapping is in action, the following properties are in play. If no wrapping happens, they don't play any role.

wrapped_attributes_per_line:

• multiple - Soft wrapping, default. Fit as many attrs per line as allowed.
• single - The first attribute stays on the first line, others are on separate lines.
• single-all - All attributes, including the first one, placed on separate lines. So, every attribute is on a separate line, and the element/tag line has no attributes left.

wrapped_attributes_indent:

1. auto -- Uses wrap_attributes_indent_size, default.
2. aligned -- All attribute lines start aligned with the first attribute. If the first attribute is on a new line ("single-all" mode of wrapped_attributes_per_line), then we use wrap_attributes_indent_size to calculate the indent.

wrapped_attributes_end:

• auto - default
• multiline - the closing > is on the separate line.

Examples

1. DEFAULT case: We soft wrap and try to fit as many attributes per line as possible, but within the line length limit.

wrap_attributes_mode = auto
wrapped_attributes_per_line = multiple
wrapped_attributes_indent = auto
wrapped_attributes_end = auto

2. Wrap and align html attributes when line reaches wrap-line-length #1285 case. Very similar to default case, but we align the wrapped attributes to be on the same indent level as the first attribute.

wrap_attributes_mode = auto
wrapped_attributes_per_line = multiple
wrapped_attributes_indent = aligned
wrapped_attributes_end = auto

3. Wrap align html attributes only when line reaches certain length #1262 case. When we wrap, we want one attribute per line, aligned with the first one.

wrap_attributes_mode = auto
wrapped_attributes_per_line = single
wrapped_attributes_indent = aligned
wrapped_attributes_end = auto

4. Current force-expand-multiline case: Force wrapping if there are more than 1 attribute, with one attribute per line, and with multi-line ending, placing the closing > on the new line.

wrap_attributes_mode = force
wrapped_attributes_per_line = single-all
wrapped_attributes_indent = auto
wrapped_attributes_end = multiline

[bitwiseman]

@vvs
Wow, thanks! That is a great proposal, really well thought out and covers all the current combinations and then some. That said, I'd like to make this PR a non-breaking change.

Could you open a new issue and put your whole comment in there?

@michaelauderer
To match approximately to @vvs's naming proposal #1285 (this PR) would use aligned-multiple and #1262 (a future PR) would use aligned-single. If that sounds good to you, please make the change and submit a PR to amend this PR.

Oh, one other thing we need in order to accept this change - update the README.md with the new option.

Thanks everyone!

[bitwiseman]

@michaelauderer
@RenaldasK
Do you have time to work on this?

[NEO97online]

@bitwiseman Unfortunately I don't, but it looks like you have it working now!

I see that this is part of v1.7.6 milestone, but there are a few other issues blocking that release.

However, since this has been a highly requested feature for a long time (we really need this fix pulled into VSCode), would it be possible to release this now and move the remaining 4 issues to v1.7.7?