[Geoserver-devel] Draft PR pom formatting

Mark_Prins · January 18, 2021, 12:39pm

hi all,
I've opened a (draft) PR to format (and check formatting) of all pom files in the project: https://github.com/geoserver/geoserver/pull/4663

It still needs some work, but some decisions should be made about things like indents etc...

for now I've fixed the invalid xml and set up 4 spaces as indent, no tabs, lineendig as LF, use UTF-8 encoding, preserve whitespace in comments...

This plugin will reformat the pom files as part of a regular maven build cycle, just like the fmt-maven-plugin does.

Mark

Andrea_Aime4 · January 18, 2021, 1:31pm

Hi Mark,

thanks! Had a look, looks like an improvement to me, the current settings seem sensible.

The PR does not really contain the reformatted files, so to see the difference one has to
check it out and run it locally. Mark, maybe you could put the result of reformat in a separate commit,
so that if changes are needed, it’s possible to wipe it and redo the formatting? (and then force push).
It would help other devs get a quick “before and after” view.

One thing that I’m a bit hesitant about, is that empty lines are getting removed. Sometimes these
lines are used to separate blocks, but the semantics of that is in our head, don’t think it’s possible
to tell the plugin about it.

As another node, I’d suggest trying to run it on the applicationContext files, they also are all over the place formatting wise.
Tried locally with the following modified config:

**/pom.xml **/applicationContext.xml **/applicationSecurityContext.xml

Love the result consistency, one thing that might not be too pleasing is that namespace declarations
are now moved all in a single line, so:

becomes:

When editing the file contents the difference is ignorable, but if one had to add a namespace, it would be annoying.

Cheers
Andrea

···

Regards, Andrea Aime

== GeoServer Professional Services from the experts! Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054 Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339 8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it ------------------------------------------------------- Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia. This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail.

Mark_Prins · January 18, 2021, 4:44pm

On 18-01-2021 14:31, Andrea Aime wrote:

Hi Mark,
thanks! Had a look, looks like an improvement to me, the current settings seem sensible.

The PR does not really contain the reformatted files, so to see the difference one has to
check it out and run it locally. Mark, maybe you could put the result of reformat in a separate commit,
so that if changes are needed, it's possible to wipe it and redo the formatting? (and then force push).
It would help other devs get a quick "before and after" view.

One thing that I'm a bit hesitant about, is that empty lines are getting removed. Sometimes these
lines are used to separate blocks, but the semantics of that is in our head, don't think it's possible
to tell the plugin about it.

As far as I can tell the document is parsed using jdom/sax and then serialized again, so whitespace without any semantic meaning is removed

https://github.com/acegi/xml-format-maven-plugin/blob/6a50b3243bff5ec602fea4c7fd6021a93e573500/src/main/java/au/com/acegi/xmlformat/FormatUtil.java#L55-L84

basically the options are limited to:
https://dom4j.github.io/javadoc/2.1.3/org/dom4j/io/OutputFormat.html

As another node, I'd suggest trying to run it on the applicationContext files, they also are all over the place formatting wise.
Tried locally with the following modified config:
                 &lt;includes&gt;
                     &lt;include&gt;\*\*/pom\.xml&lt;/include&gt;
                     &lt;include&gt;\*\*/applicationContext\.xml&lt;/include&gt;
<include>**/applicationSecurityContext.xml</include>
</includes>

3 commits now:
   1. adding the plugin
   2. addding reformatted pom files
   3. adding formatting for the Spring context files + reformatted applicationContext.xml / applicationSecurityContext.xml

I agree it would be nice to have something that limited the maximum line length in a smart manner.

Alternatively https://code.revelc.net/formatter-maven-plugin/ could be an option, it requires an Eclipse formatting profile and could much more that format/check xml files

Mark

Andrea_Aime4 · January 18, 2021, 4:54pm

As far as I can tell the document is parsed using jdom/sax and then
serialized again, so whitespace without any semantic meaning is removed

https://github.com/acegi/xml-format-maven-plugin/blob/6a50b3243bff5ec602fea4c7fd6021a93e573500/src/main/java/au/com/acegi/xmlformat/FormatUtil.java#L55-L84

basically the options are limited to:
https://dom4j.github.io/javadoc/2.1.3/org/dom4j/io/OutputFormat.html

Yep… if we wanted some “space” preservation, we’d have to use XML comments
(those are preserved AFAIK).

Alternatively https://code.revelc.net/formatter-maven-plugin/ could be
an option, it requires an Eclipse formatting profile and could much more
that format/check xml files

Yeah, it’s an option too, and I see it’s still being released. It could be used to
format XML but also json, html and css files… and java too, with more flexibility
in choosing formatting options.

Dunno, we try it out just XML and, if we like it, consider using it for more at a later
stage.

Cheers
Andrea

···

== GeoServer Professional Services from the experts! Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054 Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339 8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it ------------------------------------------------------- Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia. This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail.

Mark_Prins · January 19, 2021, 11:50am

On 18-01-2021 17:54, Andrea Aime wrote:

On Mon, Jan 18, 2021 at 5:46 PM Mark Prins <mc.prins@anonymised.com <mailto:mc.prins@anonymised.com>> wrote:

...

    Alternatively https://code.revelc.net/formatter-maven-plugin/ could be
    an option, it requires an Eclipse formatting profile and could much
    more
    that format/check xml files

Yeah, it's an option too, and I see it's still being released. It could be used to
format XML but also json, html and css files.... and java too, with more flexibility
in choosing formatting options.

Dunno, we try it out just XML and, if we like it, consider using it for more at a later
stage.

I tried to find an example Eclipse XML code formatter profile on the web, but so far no dice; I haven't run Eclipse IDE for many, many years so won't be exploring that direction.

Mark