Great, @michalmac !

Two (and a half) questions: We do have repos, having matsim-libs as parent in its pom. Some of them also with one GH action testing.
What do we need to adapt there? Are some setting of the pom inherited?
Do I need to update any of the GH action scripts?

I am asking because I plan to introduce this format also there, to have everything in the same style.
Unfortunately, it is difficult to see because this PR changes so many files that even filtering the changed files does not work properly anymore.

Great, @michalmac !

Two (and a half) questions: We do have repos, having matsim-libs as parent in its pom. Some of them also with one GH action testing. What do we need to adapt there? Are some setting of the pom inherited? Do I need to update any of the GH action scripts?

I am asking because I plan to introduce this format also there, to have everything in the same style. Unfortunately, it is difficult to see because this PR changes so many files that even filtering the changed files does not work properly anymore.

Really good questions!

• I really recommend using the same formatting across all projects if possible. It's really convenient.
• The commit is practically unreadable. Therefore, I want to create a small script to help with adapting development branches so there is only one such commit in the git history.
• If you point to the matsim POM as the parent POM, then the setting will get inherited. That means the format check will be executed in mvn verify, which is usually called in CIs. No need to change anything there. However, if you do not want to follow the matsim code format, it is possible to override the plugin settings in a child POM, it is also possible to specify -Dfmt.skip when calling mvn.

@michalmac: I have tried it out now for my other repo.

Here are two things that I noticed:

1.)
Updating the format in IntelliJ work fine. As long as only the mentioned IntelliJ plugin is installed, It only changed something when I did reformat file.
So I also installed the code style from https://github.com/google/styleguide/blob/gh-pages/intellij-java-google-style.xml in IntelliJ as code Style:

Now, I was also able to use reformat code as well.

2.)
The automatic line wrapping is IMO not so nice (independent of the reformat way: file or code):
Once it decides that a line is too long, it wraps it at any possible location and not only once and maybe again if the second line is also full. This happens at different places, e.g.: definitions of a class' implementations, constructors, calls of functions/methods - if the signature is too long, strings constructs with "+", ..., line comments
see e.g.
https://github.com/matsim-vsp/logistics/blob/6e34659f357f5d81d9a201d05dff48489dad128d/src/main/java/org/matsim/freight/logistics/LSPControlerListener.java#L44-L52
or
https://github.com/matsim-vsp/logistics/blob/6e34659f357f5d81d9a201d05dff48489dad128d/src/main/java/org/matsim/freight/logistics/example/lsp/initialPlans/ExampleTwoEchelonGrid.java#L153-L161
This does not really help to improve the readability and also makes files much longer: I have a class that has grown from 430 to 530 lines only due to the reformatting.

Similar things also happen in matsim-libs (I saw some of these changes in this PR, but am too lazy now to search for them. If wished, I can look for some of them again)

--> Do we find there a suitable solution to avoid this line wrap inflation? As far as I understood, the Google code style also allows, less line-wrappings.(?)

@michalmac: I have tried it out now for my other repo.

Here are two things that I noticed:

Thanks @kt86 for trying out the formatting.

Re 1: Thanks. I have not mentioned about importing the the Google code style, because I thought it was described in the linked IntelliJ, but apparently it is not.

Re 2: Unfortunately, there is no configurability. This has advantages and disadvantages. I would say the bigger the community, the more advantages.

IMO splitting a long list of argument/interfaces/... into individual rows helps in spotting the actual diffs in code. Moreover, it is less likely to get many lines of diff if we add/rename/remove one argument. But the code gets longer.

==

I think we need to keep in mind that there is no formatting style that will make everyone happy. However, if many of us are (very) unhappy, then probably we need to come up with a different style. I think every feedback like yours is very valuable in making the final decision.

Thank you, Michal!
I am totally in favor of a common standard and see many benefits of it. For me, as a non-expert, this Google java style seems to be a good choice because it seems to be a well-known standard, and it is effortless to integrate.
And I also like, that there is no configurability, so we will not have all the reformatting changes anymore. :)

For me personally, I can also live with that many additional lines. Nevertheless, I will try out, what happens, when not all of these line-wrappings are there, e.g., when writing new code. --> Is this something that is enforced? Or is it not enforced because both options are allowed, and it was just coming up due to the automatic reformat? ;)

So please just see it more as a note than a vote :)

And I am curious, what other people — and especially the more programming-experts — will say.

For me personally, I can also live with that many additional lines. Nevertheless, I will try out, what happens, when not all of these line-wrappings are there, e.g., when writing new code. --> Is this something that is enforced? Or is it not enforced because both options are allowed, and it was just coming up due to the automatic reformat? ;)

It is enforced. If you split lines in a different way, the format checker will complain.

So please just see it more as a note than a vote :)

Sure. Thanks.

This does not really help to improve the readability and also makes files much longer

This usually takes a while to get used to. Somehow, the overall opinion seems to be that long lines are less readable. The rust formatter as well as the very popular prettier formatter for Javascript also favor rather short lines. This also is not my favorite, especially for languages with type arguments, but then I don't care enough to invent my own format :-)

Re line length: This formatter limits it at 100, but having 2-space indentation gives a little extra spaces within that limit, compared to the current style (4-space long tabs).

This PR results in 10% more lines, but I guess it is mainly a small fraction of complex (often hard to read) multi-line sections that contribute to this 10% increase. I would even guess that having the limit increased from 100 to 140 will probably have no significant impact.

I looked a bit to find some examples in the drt contrib.

Example 1:

		drtCfg.getDrtSpeedUpParams().ifPresent(drtSpeedUpParams -> {
			bindModal(DrtSpeedUp.class).toProvider(modalProvider(
					getter -> new DrtSpeedUp(getMode(), drtSpeedUpParams, getConfig().controller(),
							getter.get(Network.class), getter.getModal(FleetSpecification.class),
							getter.getModal(DrtEventSequenceCollector.class)))).asEagerSingleton();
			addControlerListenerBinding().to(modalKey(DrtSpeedUp.class));
		});

is formatted into something more structured:

    drtCfg
        .getDrtSpeedUpParams()
        .ifPresent(
            drtSpeedUpParams -> {
              bindModal(DrtSpeedUp.class)
                  .toProvider(
                      modalProvider(
                          getter ->
                              new DrtSpeedUp(
                                  getMode(),
                                  drtSpeedUpParams,
                                  getConfig().controller(),
                                  getter.get(Network.class),
                                  getter.getModal(FleetSpecification.class),
                                  getter.getModal(DrtEventSequenceCollector.class))))
                  .asEagerSingleton();
              addControlerListenerBinding().to(modalKey(DrtSpeedUp.class));
            });

Example 2:

				.in(Singleton.class); // Not an eager binding, as taxi contrib doesn't need this implementation, but
										// would search for VrpLegFactory which is provided in the
										// DrtModeOptimizerModule if bound eagerly

The comment is located a bit unfortunately, so the automated formatting leads to the following:

        .in(
            Singleton
                .class); // Not an eager binding, as taxi contrib doesn't need this implementation,
    // but
    // would search for VrpLegFactory which is provided in the
    // DrtModeOptimizerModule if bound eagerly

There is no support for more intelligent "re-formatting" of comments. However, problems like this one will get resolved over time as we modify existing code.

Example 3:

				entriesList.forEach(entry -> writer.writeNext(
						new String[] { zone.getId(), "" + c.getX(), "" + c.getY(), "" + entry.getFirst(),
								"" + entry.getSecond() }, false));

gets formatted into something which is presumably more readable, but longer:

                entry ->
                    writer.writeNext(
                        new String[] {
                          zone.getId(),
                          "" + c.getX(),
                          "" + c.getY(),
                          "" + entry.getFirst(),
                          "" + entry.getSecond()
                        },
                        false));

Example 4:

	private record DrtLeg(Id<Request> request, double submissionTime, double departureTime, Id<Person> person, Id<DvrpVehicle> vehicle, Id<Link> fromLinkId, Coord fromCoord,
						  Id<Link> toLinkId, Coord toCoord, double waitTime, double unsharedDistanceEstimate_m, double unsharedTimeEstimate_m,
						  double arrivalTime, double fare, double earliestDepartureTime, double latestDepartureTime, double latestArrivalTime) {
	}

This long record definition starts to look more like a java class definition after formatting:

  private record DrtLeg(
      Id<Request> request,
      double submissionTime,
      double departureTime,
      Id<Person> person,
      Id<DvrpVehicle> vehicle,
      Id<Link> fromLinkId,
      Coord fromCoord,
      Id<Link> toLinkId,
      Coord toCoord,
      double waitTime,
      double unsharedDistanceEstimate_m,
      double unsharedTimeEstimate_m,
      double arrivalTime,
      double fare,
      double earliestDepartureTime,
      double latestDepartureTime,
      double latestArrivalTime) {}

My overall impression is that the formatted code is easier to read. Moreover, the whole codebase is formatted in the same way, which always increases the readability (regardless of the chosen formatter).

Has there been any progress on this? Could we maybe just start deploying it per contrib (= drt ;-) )

Has there been any progress on this? Could we maybe just start deploying it per contrib (= drt ;-) )

No final decision yet. I think everyone wants it, but there is no formatting that satisfies everyone :-) So we are a bit stuck. I guess we need to be a bit more realistic with our expectations and weigh pros and cons.

I think the easiest way to move on is to do an all-in-one-go transition:

• formatting has the viral effect: once the DRT contrib switches to this formatting, those who code in DRT would need to set up the right formatting in their IDEs and then it would impact the formatting in other contribs. At the same time others could still use old formatting, so there might be some "formatting wars" outside DRT :-)
• To avoid all open PRs to have huge diffs with the master (which basically makes them unreviewable), we need to rebase the corresponding branches on the "formatted" master (this could be done almost automatically with some script). This rebasing of branches is easier in the all-in-one-go transition.

Ok thanks, personally, I don't care about the style as long as there is one as a couple of other people here, I think. Maybe we should put it to vote at the MATSim User Meeting :)

I like the code style with shorter lines :) take my vote