Attributes resulting in long lines should be wrapped or chopped down

[winniequinn]

I have the following in a POM (due to MNG-6059):

<scm child.scm.connection.inherit.append.path="false" 
     child.scm.developerConnection.inherit.append.path="false"
     child.scm.url.inherit.append.path="false">
  ...
</scm>

After formatting, unfortunately, I am stuck with this:

<scm child.scm.connection.inherit.append.path="false" child.scm.developerConnection.inherit.append.path="false" child.scm.url.inherit.append.path="false">
  ...
</scm>

While not the end of the world, it would be nice if there were an option to wrap or chop down attributes when an element featuring them would otherwise exceed some reasonable length. This would generalize the current indentSchemaLocation option.

[winniequinn]

I took a look at the code. If there is interest, I'll happily submit a PR that adds the following:

chopDownAttributes (Default: false)
  User property: sort.chopDownAttributes
  Should all attributes be chopped down such that each appears on a new
  line. The attributes will be indented (2 * nrOfIndentSpace + 1 space)
  characters.

My example above would be formatted as such:

<scm
     child.scm.connection.inherit.append.path="false" 
     child.scm.developerConnection.inherit.append.path="false"
     child.scm.url.inherit.append.path="false">
  ...
</scm>

Essentially, when chopDownAttributes is enabled, the approach taken for indentSchemaLocation would be applied to all attributes, not just xsi:schemaLocation. This isn't necessarily ideal for elements with very short names like scm—you might prefer to keep the first attribute on the same line—but it should work well in the general case and is easy to add to the existing implementation.

[Ekryd]

Hi! This sounds interesting. A PR is always welcome.
The name 'chopDownAttributes' is really cool, but (perhaps) not the most descriptive 😅. Is the name used as a standard parameter in other plugins/situations?
How would this new functionality interact with indentSchemaLocation? Would it automatically affect xsi:schemaLocation as well?

[winniequinn]

A PR is always welcome.

Sounds good! I'll submit one soon and will reference this issue when I do.

The name 'chopDownAttributes' is really cool, but (perhaps) not the most descriptive 😅. Is the name used as a standard parameter in other plugins/situations?

"Chop down" is the terminology that JetBrains uses in their products. I'm not aware of common terminology beyond that, unfortunately. I can do oneLinePerAttribute or wrapEachAttribute instead.

How would this new functionality interact with indentSchemaLocation? Would it automatically affect xsi:schemaLocation as well?

As an attribute, yes, I think it would also have to affect xsi:schemaLocation. All the PR will really do is change the following line to also check for oneLinePerAttribute (or what have you), add the appropriate plumbing, and add some tests:

sortpom/sorter/src/main/java/sortpom/output/PatchedXMLWriter.java

Line 122 in 607e540

if (indentSchemaLocation && "xsi:schemaLocation".equals(qualifiedName)) {

[winniequinn]

Actually, I'll just do indentEachAttribute to match indentSchemaLocation (and to keep them sorted next to one another):

indentEachAttribute (Default: false)
  User property: sort.indentEachAttribute
  Should each attribute be placed on a new line. The attributes will be
  indented (2 * nrOfIndentSpace + 1 space) characters.

[winniequinn]

Just a quick note: I will keep the "2 * nrOfIndentSpace + 1 space" indentation you use for indentSchemaLocation for now, but I suspect you may use that formula because it results in xmlns and xsi:schemaLocation lining up when nrOfIndentSpace is 4. For indentEachAttribute, it probably makes sense to just do 2 * nrOfIndentSpace. (I think it makes sense to apply that universally, honestly: I use 2 for nrOfIndentSpace, and getting five spaces for xsi:schemaLocation is a bit strange.)

[Ekryd]

I believe that the 2 * nrOfIndentSpace + 1 space was due to the fact that I couldn't get rid of the 1 space at the end and nrOfIndentSpace + 1 looked very weird.
The exact indentation can be played around with later, I believe.
I was wondering if the parameter should be indentAttributes and the value would be something like EACH for all, WHEN_MULTIPLE for all except when you have only one attribute, or SCHEMA_LOCATION for only the schema location (replacing the old parameter). Perhaps there are even more values.

🤔 Naming is one of the hardest things in programming.

Don't let my suggestions interrupt your work too much. Create some code and we can look at it together.

[Ekryd]

I created the branch 412-attributes-resulting-in-long-lines-should-be-wrapped-or-chopped-down if you want to use that branch.

[Ekryd]

Hello again! I wrote some code for this as this change would affect another major build.
I have added

  /**
   * Should the xml attributes be indented. Can be either 'schemaLocation' or 'all'. The attribute
   * will be indented (2 * nrOfIndentSpace) characters. 'schemaLocation' only indents the
   * schemaLocation attribute in the project element.
   */
  @Parameter(property = "sort.indentAttribute")
  String indentAttribute;

If you are interested, please add support for 'skipFirst' which would only indent the second attribute and 'project' which would indent only everything in the project tag and ignore the rest of the attributes.

[winniequinn]

@Ekryd: Fantastic to see this! I'm sorry I wasn't able to get a PR up sooner: Life kept unexpectedly pushing back my available windows for working on other people's OSS projects.

To show my appreciation for your work, and as an apology for not being able to get to work on this myself as anticipated, I made a one-time contribution towards the project via GitHub's sponsorship feature. All the best!

[Ekryd]

Thanks a lot @winniequinn !!
Your contribution means a lot to me ❤️, I feel very happy and grateful. I hope that the feature was to your liking and please give a shout-out if it needs any tweaking.
All the best, Björn