orb icon indicating copy to clipboard operation
orb copied to clipboard

Improve formatting

Open arjantijms opened this issue 2 years ago • 12 comments

arjantijms avatar Sep 27 '23 08:09 arjantijms

This change obviously can't be reviewed in this century, but I could re-apply your process and compare results. Can you share the details how it was done?

pzygielo avatar Sep 27 '23 15:09 pzygielo

Yes, add the Eclipse EE4J formatting file to the Eclipse IDE. Select all packages, choose source -> format.

See https://github.com/eclipse-ee4j/ee4j/tree/master/codestyle

Also, if you set the diff to ignore whitespace it becomes a lot easier to see (but obviously whitespace differences are an important factor of this PR)

arjantijms avatar Sep 27 '23 16:09 arjantijms

Any progress here?

arjantijms avatar Oct 09 '23 07:10 arjantijms

I've applied ee4j_intellij_formatting.xml with IDEA and got different result.

Didn't investigate further. Either XMLs are out of sync, or there is no such thing as EE4J rules.

pzygielo avatar Oct 09 '23 14:10 pzygielo

What if we just say it improved formatting? It's much closer to the formatting of the other EE4J projects this way.

arjantijms avatar Oct 10 '23 16:10 arjantijms

I let IDEA to modify everything in imported project. That was stupid and 3200 files were changed (including html, properties, etc.) https://github.com/arjantijms/orb/compare/format...pzygielo:orb:ee4j-reformat-idea

But even for java files only - there are differences for spacing, import order, indentations.

I just haven't managed to run Eclipse with the referenced settings (yet).

pzygielo avatar Oct 12 '23 18:10 pzygielo

So I limited the changes to orbmain only (ee4j-reformat-idea-orbmain) and the results are of course much closer now: https://github.com/arjantijms/orb/compare/format..pzygielo:orb:ee4j-reformat-idea-orbmain.

But this new diff is much more consumable over the single weekend I guess.

pzygielo avatar Oct 12 '23 19:10 pzygielo

I think it's better if there was some script that would do the formatting. Then everybody could run it and compare the results. FOr example, Google formatter (https://github.com/google/google-java-format) is a tool that can executed on command line, although I don't know if it's possible to configure it to match the Eclipse formatting.

I don't like that formatting depends on an IDE. Then we get into debates which IDE is the one to use, because each IDE adds some of its formatting on top of the imported XML config for formatting.

OndroMih avatar Oct 13 '23 07:10 OndroMih

There is something to say for that indeed. Initially the IntelliJ and Eclipse files were really close, though not identical even then.

Over time each of them added additional rules, with some defaults differing from the other.

That said, even though it can be an exact science, it doesn't have to be. The basics of EE4J formatting is simply old Sun conventions, with 4 spaces for a tab, and a longer line width.

arjantijms avatar Oct 13 '23 10:10 arjantijms

Let's not overengineer it. It is the first and probably last large step. I would just check if there weren't for i loops transformed to foreach loops as it is usual trap breaking things which were invisibly already broken. Otherwise formatting should be safe.

dmatej avatar Oct 13 '23 11:10 dmatej

@pzygielo and others, is there some way to resolve this?

The current formatting is horrible, this PR makes it much nicer. There is no perfection, but just a step in the right direction.

arjantijms avatar Oct 21 '23 18:10 arjantijms

@pzygielo just a ping. Any progress here?

arjantijms avatar Dec 05 '23 00:12 arjantijms