Provide code formatter settings for automatic code formatting

[eclipse]

Provide code formatter settings for automatic code formatting

[kevinherron]

I did also start trying to put together a Wiki page on the topic: https://github.com/eclipse/milo/wiki/Code-Style

If it would help, I could also export my Intellij IDEA settings (I know, I know...). I'm not really keen on hitting the project with another mass formatting wave.

[kevinherron]

I've started working on a checkstyle configuration. I think I read that Eclipse may be able to generate formatting settings based on a checkstyle config, but regardless, this should be a step in the right direction.

[ctron]

I completely understand ... few topic qualify for more discussions

Since I am an Eclipse fan, I am not starting to argue about the IDE, you can think of all kind of reasons I could come up with yourself 😉 Unfortunately Eclipse seems to ignore the "tabs to spaces" setting unless you have code formatting enabled. 🤦 At least I was not able to manage.

My biggest two issues with creating an Eclipse formatter was that a) In your current style it seems that there is a split between the lambda parameter declaration and the lambda body. Which results in larger methods calls to be formatted like this:

callToFunction ( foo -> {
   bar ();
   return;
});

While Eclipse seem to be only able to place each function argument (and the lambda is one as well) on a new line:

callToFunction (
   foo -> {
      bar ();
      return;
   }
);

Of course only for long method calls, not as I did it here. Just an example. To me it doesn't look that bad, but of course, this is highly personal.

My second issue b) is easier to solve I guess. There seem to be many places where the code style
simply is not applied. So I do think that the source code should be cleaned up anyway, at least once. Whether it is IntellJ, Eclipse, CheckStyle or FooBarHipsterFormatter, before making a first release, there should be a way to do it automatically and in full. With a simple explanation like "do this" -> "get that". If it takes more time to figure out the code formatting style than to write a patch, then this will hurt your project. Or it costs your time re-formatting patches.

I think it is totally ok, thought for an Eclipse project a bit awkward, to check in your IntelliJ code formatter file and simply point the wiki to that. I will then run them through IntelliJ before committing to Git 😁

[kevinherron]

Yes, I realized as I implemented checkstyle in the checkstyle branch that the existing code is not as pristine as I thought... so I've done a lot of formatting to make checkstyle pass.

Is Eclipse capable of only formatting the selected part of a file versus the entire thing? In the case of your lambda example, I don't think I actually care which style is used, and neither should the checkstyle plugin. A lot of leeway is given when it comes to line wrapping because the primary aim is readability (objective as it is) and there are more situations than a tool could ever police.

What I do care about is that the Eclipse formatter doesn't go wildly changing every instance of one format to the other in existing code. There's nothing worse than a diff that has more formatting changes than its actual intended content.

[ctron]

Yes Eclipse can only format "the current element". Which might be a bit more that "the current edit".

From what it have learnt, people tend to forget that however. As it it the case in your project ;-)

So forcing the IDE to just apply the formatter with every save, fixes this problem. But I admit, the formatter has to produce stable results and readable results then.

Also I think that doing such a thing before a first release is easier than doing it afterwards.

BTW, I also do think that we are talking about improvements on a very high level. Your project is in really good shape!!

[kevinherron]

Well, they'll be forced not to forget it, because once our Hudson instance is running it will reject PRs that don't pass checkstyle! 😄

[ctron]

That will work as well 😁

[kevinherron]

@ctron
I want to merge the checkstyle branch soon but there's one last thing I'm unsure about. The EPL file header I copied from somewhere had tabs in it, so one of the fixes I made in the checkstyle branch was to replace tabs with spaces... but now I'm not sure if the indentation in the header matches what the "standard" header is because I can't find a definitive reference anywhere.

Is there a standard?

[ctron]

Yes there is:

/*******************************************************************************
 * Copyright (c) 2006, 2015 IBM Corporation and others.
 * All rights reserved. This program and the accompanying materials
 * are made available under the terms of the Eclipse Public License v1.0
 * which accompanies this distribution, and is available at
 * http://www.eclipse.org/legal/epl-v10.html
 *
 * Contributors:
 *     IBM Corporation - initial API and implementation
 *     Brad Reynolds - bug 159768
 *     Boris Bokowski - bug 218269
 *     Matthew Hall - bug 218269, 254524, 146906, 281723
 ******************************************************************************/

Looking in the org.eclipse.platform repository mostly is the best approach.

However the first entry is, sometimes, only the company since they could not trace back who did what. Sometimes you see the email address after the name in <>.

This is also the header which is default in the Eclipse copyright helper plugin.

I do know your case is a little bit different because you are dual licensed. But you can just improvise here ;-)

[kevinherron]

Fixed in PR #68