Re: Deterministic formatter
Hi all, On Wed, 8 Nov 2023 at 09:05, Volkan Yazıcı wrote: > I completely agree with Matt. With or without IDE plugins, we run `./mvnw > spotless:apply` anyway. Hence, lack of Eclipse support is not a blocker, > IMO. Gary is covered. > > +1 deterministic formatter (don't have an opinion on Palantir-vs-Google) > > Piotr, it has been two months or so since we are discussing this. No > objections so far. Please go ahead and implement this. It will help a lot > for sync'ing `2.x` and `3.x`. I submitted a PR against `palantir-java-formatter`: https://github.com/palantir/palantir-java-format/pull/949 It's an independent Maven project, so it can be used to generate an Eclipse plugin that "works for me". Gary can you check if it works for you? It might take me a while to integrate that into Palantir's Gradle build. If you have mad Gradle skills, feel free to get a crack at it. As far as I can tell, the only "hard" operation is retrieving the `org.eclipse.jdt.core` artifact from a P2 repository to use in the compilation step. The remaining steps are copying a bunch of resources and three dependency libraries into the JAR. +1 I consider this task accomplished and I'll start deploying the Palantir formatter to our repos next week. Piotr
Re: Deterministic formatter
On Wed, Nov 8, 2023, at 09:04, Volkan Yazıcı wrote: > I completely agree with Matt. With or without IDE plugins, we run `./mvnw > spotless:apply` anyway. Hence, lack of Eclipse support is not a blocker, > IMO. Gary is covered. > > +1 deterministic formatter (don't have an opinion on Palantir-vs-Google) > > Piotr, it has been two months or so since we are discussing this. No > objections so far. Please go ahead and implement this. It will help a lot > for sync'ing `2.x` and `3.x`. > +1, I like the palantir formatter the most for the same reasons Matt mentioned. Since it can be done using Maven, I don't know why an IDE should block it. Christian > On Tue, Nov 7, 2023 at 7:10 PM Matt Sicker wrote: > >> In the worst case scenario, we can still format from maven before >> committing (which is what I used to do before finding that there were >> IntelliJ plugins for this). In fact, I have to do that all the time lately >> anyways by running `mvn spotless:apply`. >> >> > On Nov 6, 2023, at 9:00 AM, Carter Kozak >> wrote: >> > >> > I'd be happy to review+release changes to get the eclipse plugin in that >> repo into a good place as long as it doesn't make the build process a great >> deal more complicated. We don't have many folks internally using eclipse so >> support hasn't been a priority, but the easier it is to use across common >> toolchains, the better! >> > >> > -ck >> > >> > On Mon, Nov 6, 2023, at 06:55, Piotr P. Karwasz wrote: >> >> Hi Gary, >> >> >> >> On Mon, 6 Nov 2023 at 11:45, Gary Gregory >> wrote: >> >>> >> >>> Well, I use Eclipse, so... I won't be using whatever this does or when >> it >> >>> does it. >> >> >> >> What version of Eclipse do you use? The Eclipse plugin is one class, I >> >> can probably fix it, compile it and release it. >> >> >> >> Piotr >> >> >> >>
Re: Deterministic formatter
I completely agree with Matt. With or without IDE plugins, we run `./mvnw spotless:apply` anyway. Hence, lack of Eclipse support is not a blocker, IMO. Gary is covered. +1 deterministic formatter (don't have an opinion on Palantir-vs-Google) Piotr, it has been two months or so since we are discussing this. No objections so far. Please go ahead and implement this. It will help a lot for sync'ing `2.x` and `3.x`. On Tue, Nov 7, 2023 at 7:10 PM Matt Sicker wrote: > In the worst case scenario, we can still format from maven before > committing (which is what I used to do before finding that there were > IntelliJ plugins for this). In fact, I have to do that all the time lately > anyways by running `mvn spotless:apply`. > > > On Nov 6, 2023, at 9:00 AM, Carter Kozak > wrote: > > > > I'd be happy to review+release changes to get the eclipse plugin in that > repo into a good place as long as it doesn't make the build process a great > deal more complicated. We don't have many folks internally using eclipse so > support hasn't been a priority, but the easier it is to use across common > toolchains, the better! > > > > -ck > > > > On Mon, Nov 6, 2023, at 06:55, Piotr P. Karwasz wrote: > >> Hi Gary, > >> > >> On Mon, 6 Nov 2023 at 11:45, Gary Gregory > wrote: > >>> > >>> Well, I use Eclipse, so... I won't be using whatever this does or when > it > >>> does it. > >> > >> What version of Eclipse do you use? The Eclipse plugin is one class, I > >> can probably fix it, compile it and release it. > >> > >> Piotr > >> > >
Re: Deterministic formatter
In the worst case scenario, we can still format from maven before committing (which is what I used to do before finding that there were IntelliJ plugins for this). In fact, I have to do that all the time lately anyways by running `mvn spotless:apply`. > On Nov 6, 2023, at 9:00 AM, Carter Kozak wrote: > > I'd be happy to review+release changes to get the eclipse plugin in that repo > into a good place as long as it doesn't make the build process a great deal > more complicated. We don't have many folks internally using eclipse so > support hasn't been a priority, but the easier it is to use across common > toolchains, the better! > > -ck > > On Mon, Nov 6, 2023, at 06:55, Piotr P. Karwasz wrote: >> Hi Gary, >> >> On Mon, 6 Nov 2023 at 11:45, Gary Gregory wrote: >>> >>> Well, I use Eclipse, so... I won't be using whatever this does or when it >>> does it. >> >> What version of Eclipse do you use? The Eclipse plugin is one class, I >> can probably fix it, compile it and release it. >> >> Piotr >>
Re: Deterministic formatter
I'd be happy to review+release changes to get the eclipse plugin in that repo into a good place as long as it doesn't make the build process a great deal more complicated. We don't have many folks internally using eclipse so support hasn't been a priority, but the easier it is to use across common toolchains, the better! -ck On Mon, Nov 6, 2023, at 06:55, Piotr P. Karwasz wrote: > Hi Gary, > > On Mon, 6 Nov 2023 at 11:45, Gary Gregory wrote: > > > > Well, I use Eclipse, so... I won't be using whatever this does or when it > > does it. > > What version of Eclipse do you use? The Eclipse plugin is one class, I > can probably fix it, compile it and release it. > > Piotr >
Re: Deterministic formatter
The latest: 4.29.0 Gary On Mon, Nov 6, 2023, 6:55 AM Piotr P. Karwasz wrote: > Hi Gary, > > On Mon, 6 Nov 2023 at 11:45, Gary Gregory wrote: > > > > Well, I use Eclipse, so... I won't be using whatever this does or when it > > does it. > > What version of Eclipse do you use? The Eclipse plugin is one class, I > can probably fix it, compile it and release it. > > Piotr >
Re: Deterministic formatter
Hi Gary, On Mon, 6 Nov 2023 at 11:45, Gary Gregory wrote: > > Well, I use Eclipse, so... I won't be using whatever this does or when it > does it. What version of Eclipse do you use? The Eclipse plugin is one class, I can probably fix it, compile it and release it. Piotr
Re: Deterministic formatter
Well, I use Eclipse, so... I won't be using whatever this does or when it does it. Gary On Mon, Nov 6, 2023, 3:00 AM Piotr P. Karwasz wrote: > Hi Matt, > > On Fri, 3 Nov 2023 at 19:44, Matt Sicker wrote: > > > > Alright, after looking at the differences, I’m strongly in favor of the > Palantir version. The differences in how it handles lambdas solves one of > the main complaints I ever had about the Google version. > > The only disadvantage of the Palantir version is that it does not > provide/support an Eclipse plugin: > https://github.com/palantir/palantir-java-format/issues/833 > > The code for Eclipse is still there, it just isn't published anywhere > (AFAIK): > https://github.com/palantir/palantir-java-format/tree/develop/eclipse_plugin > > Gary, would it be a big inconvenience using Palantir instead of the > Google AOSP format? > > Piotr >
Re: Deterministic formatter
Hi Matt, On Fri, 3 Nov 2023 at 19:44, Matt Sicker wrote: > > Alright, after looking at the differences, I’m strongly in favor of the > Palantir version. The differences in how it handles lambdas solves one of the > main complaints I ever had about the Google version. The only disadvantage of the Palantir version is that it does not provide/support an Eclipse plugin: https://github.com/palantir/palantir-java-format/issues/833 The code for Eclipse is still there, it just isn't published anywhere (AFAIK): https://github.com/palantir/palantir-java-format/tree/develop/eclipse_plugin Gary, would it be a big inconvenience using Palantir instead of the Google AOSP format? Piotr
Re: Deterministic formatter
Alright, after looking at the differences, I’m strongly in favor of the Palantir version. The differences in how it handles lambdas solves one of the main complaints I ever had about the Google version. > On Oct 25, 2023, at 11:47 PM, Piotr P. Karwasz > wrote: > > Hi Matt, > > On Thu, 26 Oct 2023 at 01:03, Matt Sicker wrote: >> >> Seems reasonable to me. I can’t really tell what the difference is between >> the two resulting outputs, though. > > There was no difference (I must have used the same Spotless config > twice), so I corrected it. > > Anyway the differences are very subtle and are mainly in the wrapping > algorithm: > > https://github.com/palantir/palantir-java-format > > Piotr
Re: Deterministic formatter
Hi Matt, On Thu, 26 Oct 2023 at 01:03, Matt Sicker wrote: > > Seems reasonable to me. I can’t really tell what the difference is between > the two resulting outputs, though. There was no difference (I must have used the same Spotless config twice), so I corrected it. Anyway the differences are very subtle and are mainly in the wrapping algorithm: https://github.com/palantir/palantir-java-format Piotr
Re: Deterministic formatter
Seems reasonable to me. I can’t really tell what the difference is between the two resulting outputs, though. > On Oct 25, 2023, at 3:06 PM, Piotr P. Karwasz wrote: > > Hi all, > > On Thu, 21 Sept 2023 at 07:15, Piotr P. Karwasz > wrote: >> Spotless supports many formatter plugins, the main ones being Eclipse, >> Google and Palantir formatters. All three are IDE agnostic. > > I created two PRs using the Google (AOSP variant)[1] and Palantir[2] > formatter. > > If we were to adopt one of these we are facing around 15% of changes > in LOCs (mostly wrapping of long lines). This seems acceptable to me, > what do you think? > > Piotr > > [1] https://github.com/apache/logging-log4j2/pull/1910 > [2] https://github.com/apache/logging-log4j2/pull/1911
Re: Deterministic formatter
Hi all, On Thu, 21 Sept 2023 at 07:15, Piotr P. Karwasz wrote: > Spotless supports many formatter plugins, the main ones being Eclipse, > Google and Palantir formatters. All three are IDE agnostic. I created two PRs using the Google (AOSP variant)[1] and Palantir[2] formatter. If we were to adopt one of these we are facing around 15% of changes in LOCs (mostly wrapping of long lines). This seems acceptable to me, what do you think? Piotr [1] https://github.com/apache/logging-log4j2/pull/1910 [2] https://github.com/apache/logging-log4j2/pull/1911
Re: Deterministic formatter
Hi Gary, On Wed, 20 Sept 2023 at 16:23, Gary Gregory wrote: > Hmm... interesting but I can't help but think that we are making our > build more complex by jumping through hoops to fix problems of our own > making: We already use spotless in the builds. Now we want to use > OpenRewrite but that makes a "mess", so now we want to use _another_ > plugin to fix that, that in itself is a mess IMO. My favorite way to > deal with formatting and refactoring is to use Eclipse which has a > large set of formatting configuration options and clean-ups. Not > everyone uses Eclipse, so sharing an Eclipse or any other IDE > formatter config might be pointless. I think we need to keep > refactorings OUT of builds, formatting is already weird enough. Sure, I don't plan to add refactorings into the builds, but if we do use OpenRewrite locally we need a way to fix the formatting problems caused by the refactoring. The current Spotless configuration does not do it. Spotless supports many formatter plugins, the main ones being Eclipse, Google and Palantir formatters. All three are IDE agnostic. I like the idea of using a non-configurable one, since it is a binary choice, but the Eclipse formatter is a great tool with literally hundreds of options. As long as we don't have to discuss every option and the options that bloat the line length (like "align field initializers") are off, I am fine with it. Could you maybe add your formatter's configuration to `logging-parent` so that we can evaluate how it formats code? Piotr PS: I can submit an option to Spotless, so that package `foo` is ordered before class `FooBar` (as the Eclipse IDE does it). Spotless right now only uses `String.compareTo`.
Re: Deterministic formatter
We use the Google Java formatter in Spinnaker, and while I know some of my teammates despise the particular code style chosen there, my preference is consistency over anything. A deterministic formatter would be best for avoiding merge conflicts. If we adopt a formatter, as long as it’s IDE-agnostic (like the GJF one is), then I’m alright with it. > On Sep 20, 2023, at 8:29 AM, Gary Gregory wrote: > > Hmm... interesting but I can't help but think that we are making our > build more complex by jumping through hoops to fix problems of our own > making: We already use spotless in the builds. Now we want to use > OpenRewrite but that makes a "mess", so now we want to use _another_ > plugin to fix that, that in itself is a mess IMO. My favorite way to > deal with formatting and refactoring is to use Eclipse which has a > large set of formatting configuration options and clean-ups. Not > everyone uses Eclipse, so sharing an Eclipse or any other IDE > formatter config might be pointless. I think we need to keep > refactorings OUT of builds, formatting is already weird enough. > > Gary > > On Wed, Sep 20, 2023 at 1:45 AM Piotr P. Karwasz > wrote: >> >> Hi, >> >> Right now our Spotless configuration just specifies the lines endings, >> forbids tabs and sorts the imports. If we want to apply some >> OpenRewrite rule to the codebase (e.g. migrate all string >> concatenations to parameterized logging or migrate JUnit4 to Junit5), >> we need a deterministic formatter to clean up the mess OpenRewrite >> leaves behind. >> >> On my personal projects I've bee playing with Google Java format[1] >> for some time and it basically corresponds to what we already use >> except: >> >> * it has a non-configurable indentation of 2 spaces. >> >> There is also a Palantir Java format[2], which is as far as I can tell >> also non-configurable (which is a feature, not a problem). Both have >> maintained Eclipse and IntelliJ plugins. >> >> On the other hand of the spectrum there is an Eclipse formatter, which >> is fully configurable and apparently has an IntelliJ plugin[3]. This >> is the one I used at my dayjob, but it lead to months of discussions, >> complains about the settings and a list of trick and tips on how to >> change the formatting with comments. We also had a minor problem with >> import sorting: >> >> * the formatter does not sort imports, so we used Spotless to do it. >> However Spotless and the Eclipse IDE use a different collation, which >> generated some conflicts. >> >> What do you think about enforcing a deterministic formatter on our code? >> >> Piotr >> >> [1] https://github.com/google/google-java-format >> [2] https://github.com/palantir/palantir-java-format >> [3] >> https://plugins.jetbrains.com/plugin/6546-adapter-for-eclipse-code-formatter
Re: Deterministic formatter
Hmm... interesting but I can't help but think that we are making our build more complex by jumping through hoops to fix problems of our own making: We already use spotless in the builds. Now we want to use OpenRewrite but that makes a "mess", so now we want to use _another_ plugin to fix that, that in itself is a mess IMO. My favorite way to deal with formatting and refactoring is to use Eclipse which has a large set of formatting configuration options and clean-ups. Not everyone uses Eclipse, so sharing an Eclipse or any other IDE formatter config might be pointless. I think we need to keep refactorings OUT of builds, formatting is already weird enough. Gary On Wed, Sep 20, 2023 at 1:45 AM Piotr P. Karwasz wrote: > > Hi, > > Right now our Spotless configuration just specifies the lines endings, > forbids tabs and sorts the imports. If we want to apply some > OpenRewrite rule to the codebase (e.g. migrate all string > concatenations to parameterized logging or migrate JUnit4 to Junit5), > we need a deterministic formatter to clean up the mess OpenRewrite > leaves behind. > > On my personal projects I've bee playing with Google Java format[1] > for some time and it basically corresponds to what we already use > except: > > * it has a non-configurable indentation of 2 spaces. > > There is also a Palantir Java format[2], which is as far as I can tell > also non-configurable (which is a feature, not a problem). Both have > maintained Eclipse and IntelliJ plugins. > > On the other hand of the spectrum there is an Eclipse formatter, which > is fully configurable and apparently has an IntelliJ plugin[3]. This > is the one I used at my dayjob, but it lead to months of discussions, > complains about the settings and a list of trick and tips on how to > change the formatting with comments. We also had a minor problem with > import sorting: > > * the formatter does not sort imports, so we used Spotless to do it. > However Spotless and the Eclipse IDE use a different collation, which > generated some conflicts. > > What do you think about enforcing a deterministic formatter on our code? > > Piotr > > [1] https://github.com/google/google-java-format > [2] https://github.com/palantir/palantir-java-format > [3] > https://plugins.jetbrains.com/plugin/6546-adapter-for-eclipse-code-formatter
Deterministic formatter
Hi, Right now our Spotless configuration just specifies the lines endings, forbids tabs and sorts the imports. If we want to apply some OpenRewrite rule to the codebase (e.g. migrate all string concatenations to parameterized logging or migrate JUnit4 to Junit5), we need a deterministic formatter to clean up the mess OpenRewrite leaves behind. On my personal projects I've bee playing with Google Java format[1] for some time and it basically corresponds to what we already use except: * it has a non-configurable indentation of 2 spaces. There is also a Palantir Java format[2], which is as far as I can tell also non-configurable (which is a feature, not a problem). Both have maintained Eclipse and IntelliJ plugins. On the other hand of the spectrum there is an Eclipse formatter, which is fully configurable and apparently has an IntelliJ plugin[3]. This is the one I used at my dayjob, but it lead to months of discussions, complains about the settings and a list of trick and tips on how to change the formatting with comments. We also had a minor problem with import sorting: * the formatter does not sort imports, so we used Spotless to do it. However Spotless and the Eclipse IDE use a different collation, which generated some conflicts. What do you think about enforcing a deterministic formatter on our code? Piotr [1] https://github.com/google/google-java-format [2] https://github.com/palantir/palantir-java-format [3] https://plugins.jetbrains.com/plugin/6546-adapter-for-eclipse-code-formatter
