Thanks for looking into this problem Matthias. The patched version also works for me. +1 for what Jark proposed.
Cheers, Till On Sun, Jan 24, 2021 at 3:50 PM Matthias Pohl <matth...@ververica.com> wrote: > Maybe, it's worth a try: I created issue #560 [1] for this. Let's see what > happens. > > [1] https://github.com/google/google-java-format/issues/560 > > On Sun, Jan 24, 2021 at 4:17 AM Jark Wu <imj...@gmail.com> wrote: > >> Hi Matthias, >> >> I also have the same problem when creating a new Java class. This is >> quite annoying. Thanks for looking into it and providing a patch. The >> patched plugin works well for me. >> >> Regarding the next actions, is it possible to backport this fix to >> google-java-format 1.7.0 series, and release a new version for that? The >> latest version of 1.7.0.5 is released in Apr 2020. If this doesn't work, >> option #2 also sounds good to me, because users need to download the plugin >> anyway. >> >> Best, >> Jark >> >> >> >> On Sun, 24 Jan 2021 at 03:32, Matthias Pohl <matth...@ververica.com> >> wrote: >> >>> // With this one I am curious now how many of you had the same issue >>> without complaining: In the engine team there were 4 out of 4. >>> >>> It's about the error dialog that pops up when creating a new Java class >>> file. Additionally, the Java class is generated but is not formatted >>> correctly. Reformatting the file helps. Confirming the dialog is annoying, >>> though. I started looking into the issue and found an >>> UnsupportedOperationException that led me to the actual cause: A bug in the >>> google-java-format plugin version 1.7.0.5 is causing this behavior. I >>> provided a more detailed description of my findings in FLINK-21106 [1] >>> including a compiled version of the patched plugin. >>> >>> I want to open the discussion on how we want to deal with it. I see >>> three options right now: >>> 1. We leave it as it is right now as we consider this to be a minor >>> thing. >>> 2. We provide the patched google-java-format plugin as part of the docs. >>> 3. We upgrade to Java 11 to be able to upgrade the google-java-format >>> plugin as it was already mentioned earlier in the thread. >>> >>> None of the above options seem to be the right one to go for. Any >>> thoughts on this? >>> >>> Best, >>> Matthias >>> >>> [1] https://issues.apache.org/jira/browse/FLINK-21106 >>> >>> >>> >>> On Wed, Dec 30, 2020 at 11:05 AM Chesnay Schepler <ches...@apache.org> >>> wrote: >>> >>>> 1) No, it is not possible to exclude certain code blocks from >>>> formatting. >>>> There is a workaround though for this case; you can add en empty >>>> comment >>>> (//) to the end of a line to prevent subsequent lines from being added >>>> to it. >>>> https://github.com/google/google-java-format/issues/137 >>>> >>>> Note that any spotless-specific features would like not help us anyway, >>>> unless we'd be fine with not using the IntelliJ plugin. >>>> >>>> 2) The code style has not been updated yet. >>>> The indent choices with the google-java-format is either 2 spaces for >>>> everything, or 4 spaces + 8 spaces for continuations. >>>> In other words, 4 spaces is simply not an option. >>>> >>>> On 12/30/2020 9:09 AM, Jark Wu wrote: >>>> > Hi, >>>> > >>>> > I have played with the format plugin these days and found some >>>> problems. >>>> > Maybe some of them are personal taste. >>>> > >>>> > 1) Is it possible to disable auto-format for some code blocks? >>>> > For example, the format of code generation >>>> > StructuredObjectConverter#generateCode is manually >>>> > adjusted for readability. However, the format plugin breaks it and >>>> it's >>>> > hard to read now. >>>> > See before[1] and after[2]. cc @Timo Walther <twal...@apache.org> >>>> > >>>> > 2) Using 4 spaces or 8 spaces for continuation indent? >>>> > AOSP uses 8 spaces for continuation indent. >>>> > However, Flink Code Style suggests "Each new line should have one >>>> extra >>>> > indentation relative to >>>> > the line of the called entity" which means 4 spaces. >>>> > Personally, I think 4 spaces may be more friendly for Java lambdas. >>>> An >>>> > example: >>>> > >>>> > 8 spaces: >>>> > >>>> > wrapClassLoader( >>>> > () -> >>>> > environment >>>> > .getModules() >>>> > .forEach( >>>> > (name, entry) -> >>>> > modules.put( >>>> > name, >>>> > createModule( >>>> > >>>> entry.asMap(), >>>> > classLoader)))); >>>> > >>>> > 4 spaces: >>>> > >>>> > wrapClassLoader( >>>> > () -> >>>> > environment >>>> > .getModules() >>>> > .forEach( >>>> > (name, entry) -> >>>> > modules.put(name, createModule(entry.asMap(), >>>> > classLoader)))); >>>> > >>>> > >>>> > >>>> > Best, >>>> > Jark >>>> > >>>> > [1]: >>>> > >>>> https://github.com/apache/flink/blob/8fc6eaead00d6e98535874b0a137bc59716d260a/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/data/conversion/StructuredObjectConverter.java#L169 >>>> > [2]: >>>> > >>>> https://github.com/apache/flink/blob/master/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/data/conversion/StructuredObjectConverter.java#L170 >>>> > [3]: >>>> > >>>> https://flink.apache.org/contributing/code-style-and-quality-formatting.html#breaking-the-lines-of-too-long-statements >>>> > >>>> > On Tue, 29 Dec 2020 at 19:19, Chesnay Schepler <ches...@apache.org> >>>> wrote: >>>> > >>>> >> I have filed FLINK-20803 >>>> >> <https://issues.apache.org/jira/browse/FLINK-20803> for the issue >>>> that >>>> >> Till raised. >>>> >> >>>> >> google-java-format 1.8 and above require Java 11 to /run/, so we'll >>>> >> stick with 1.7 for the time being. >>>> >> This requires a manual installation of the google-java-format plugin, >>>> >> and remembering to not update the plugin (urgh...). >>>> >> >>>> >> Long term we may want to think about requiring Java 11 for >>>> /development/ >>>> >> (*not* usage) of Flink. >>>> >> >>>> >> On 12/29/2020 12:13 PM, Till Rohrmann wrote: >>>> >>> I might have found a problem: >>>> >>> >>>> >>> In my current setup I am using the google-java-format plugin version >>>> >>> 1.9.0.0 which uses google-java-format 1.9.0. In our spotless >>>> >> configuration >>>> >>> we are using google-java-format 1.7.0, however. The result is that >>>> >> spotless >>>> >>> formats my code differently than IntelliJ. The following snippet >>>> shows >>>> >> the >>>> >>> difference: >>>> >>> >>>> >>> IntelliJ formatting with google-java-format 1.9.0: >>>> >>> >>>> >>> return >>>> >>> >>>> >> >>>> Optional.ofNullable(archivedExecutionGraph.getAllVertices().get(jobVertexId)) >>>> >>> .map( >>>> >>> accessExecutionJobVertex -> >>>> >>> >>>> >>> Arrays.asList(accessExecutionJobVertex.getTaskVertices())) >>>> >>> .orElse(Collections.emptyList()) >>>> >>> .stream() >>>> >>> .map(AccessExecutionVertex::getCurrentExecutionAttempt) >>>> >>> .collect(Collectors.toList()); >>>> >>> >>>> >>> Spotless formatting with google-java-format 1.7.0: >>>> >>> >>>> >>> return >>>> >>> >>>> >> >>>> Optional.ofNullable(archivedExecutionGraph.getAllVertices().get(jobVertexId)) >>>> >>> .map( >>>> >>> accessExecutionJobVertex -> >>>> >>> >>>> >>> Arrays.asList(accessExecutionJobVertex.getTaskVertices())) >>>> >>> .orElse(Collections.emptyList()).stream() >>>> >>> .map(AccessExecutionVertex::getCurrentExecutionAttempt) >>>> >>> .collect(Collectors.toList()); >>>> >>> >>>> >>> Note that the .stream() method is in the same line as .orElse(). >>>> >>> >>>> >>> I think this raises a bit the question which versions do we want to >>>> use? >>>> >>> Manually installing google-java-format plugin version 1.7.0.5 >>>> solved the >>>> >>> problem for me. >>>> >>> >>>> >>> Cheers, >>>> >>> Till >>>> >>> >>>> >>> On Tue, Dec 29, 2020 at 11:58 AM Flavio Pompermaier < >>>> >> pomperma...@okkam.it> >>>> >>> wrote: >>>> >>> >>>> >>>> Thanks Aljoscha and Chesnay for this small but important >>>> improvement! >>>> >>>> In the new year writing new Flink features will be funnier than >>>> ever ;) >>>> >>>> >>>> >>>> On Tue, Dec 29, 2020 at 9:58 AM Till Rohrmann < >>>> trohrm...@apache.org> >>>> >>>> wrote: >>>> >>>> >>>> >>>>> Thanks a lot for this effort Aljoscha and Chesnay! Finally we >>>> have a >>>> >>>> common >>>> >>>>> code style :-) >>>> >>>>> >>>> >>>>> Cheers, >>>> >>>>> Till >>>> >>>>> >>>> >>>>> On Tue, Dec 29, 2020 at 7:32 AM Matthias Pohl < >>>> matth...@ververica.com> >>>> >>>>> wrote: >>>> >>>>> >>>> >>>>>> Yes, thanks for driving this, Aljoscha. ...and Chesnay as well >>>> for >>>> >>>>> helping >>>> >>>>>> to finalize it. >>>> >>>>>> Good job. >>>> >>>>>> >>>> >>>>>> Matthias >>>> >>>>>> >>>> >>>>>> On Tue, Dec 29, 2020 at 5:23 AM Jark Wu <imj...@gmail.com> >>>> wrote: >>>> >>>>>> >>>> >>>>>>> Thanks Aljoscha and Chesnay for the great work! >>>> >>>>>>> >>>> >>>>>>> Best, >>>> >>>>>>> Jark >>>> >>>>>>> >>>> >>>>>>> On Tue, 29 Dec 2020 at 11:11, Xintong Song < >>>> tonysong...@gmail.com> >>>> >>>>>> wrote: >>>> >>>>>>>> Great! Thanks Aljoscha and Chesnay for driving this. >>>> >>>>>>>> >>>> >>>>>>>> Thank you~ >>>> >>>>>>>> >>>> >>>>>>>> Xintong Song >>>> >>>>>>>> >>>> >>>>>>>> >>>> >>>>>>>> >>>> >>>>>>>> On Tue, Dec 29, 2020 at 1:28 AM Chesnay Schepler < >>>> >>>> ches...@apache.org >>>> >>>>>>>> wrote: >>>> >>>>>>>> >>>> >>>>>>>>> Hello everyone, >>>> >>>>>>>>> >>>> >>>>>>>>> I have just merged the commits for FLINK-20651 >>>> >>>>>>>>> <https://issues.apache.org/jira/browse/FLINK-20651> to >>>> master, >>>> >>>>>>>>> release-1.12 and release-11, which enforces new formatting >>>> rules >>>> >>>>>> using >>>> >>>>>>>>> the spotless plugin, following the google-java-format. >>>> >>>>>>>>> >>>> >>>>>>>>> This change touched every single java file in the repository, >>>> >>>>>>>>> predominantly due to switching from tabs to spaces. This >>>> implies >>>> >>>>> that >>>> >>>>>>>>> every PR and WIP branch will require a rebase. >>>> >>>>>>>>> >>>> >>>>>>>>> >>>> >>>>>>>>> Most of the changes were done by a single commit, which you >>>> can >>>> >>>>>> exclude >>>> >>>>>>>>> from git blame by configuring git as follows (note that this >>>> >>>>> requires >>>> >>>>>>>>> git 2.23+, and also works for IntelliJ): >>>> >>>>>>>>> >>>> >>>>>>>>> git config blame.ignoreRevsFile .git-blame-ignore-revs >>>> >>>>>>>>> >>>> >>>>>>>>> >>>> >>>>>>>>> You can setup the IDE to follow the new code style as follows: >>>> >>>>>>>>> >>>> >>>>>>>>> 1. Install the google-java-format plugin >>>> >>>>>>>>> <https://plugins.jetbrains.com/plugin/8527-google-java-format >>>> > >>>> >>>> and >>>> >>>>>>>>> enable it for the project >>>> >>>>>>>>> 2. In the plugin settings, change the code style to "AOSP" >>>> >>>> (4-space >>>> >>>>>>>>> indents) >>>> >>>>>>>>> 3. Install the Save Actions plugin >>>> >>>>>>>>> <https://plugins.jetbrains.com/plugin/7642-save-actions> >>>> >>>>>>>>> 4. Enable the plugin, along with "Optimize imports" and >>>> "Reformat >>>> >>>>>> file" >>>> >>>>>>>>> To manually apply the formatting, run: >>>> >>>>>>>>> >>>> >>>>>>>>> mvn com.diffplug.spotless:spotless-maven-plugin:apply >>>> >>>>>>>>> >>>> >>>>>>>>> >>>> >>>>>>>>> Please be on the lookout for any suspicious formatting, >>>> outdated >>>> >>>>>>>>> instructions or other inconveniences. >>>> >>>>>>>>> >>>> >>>>>>>>> >>>> >>>>>>>>> Finally, a big thank you to Aljoscha for pushing this topic >>>> and >>>> >>>>>> finally >>>> >>>>>>>>> bringing it to an end. >>>> >>>>>>>>> >>>> >>>>>>>>> >>>> >> >>>> >>> > > -- > > Matthias Pohl | Engineer > > Follow us @VervericaData Ververica <https://www.ververica.com/> > > -- > > Join Flink Forward <https://flink-forward.org/> - The Apache Flink > Conference > > Stream Processing | Event Driven | Real Time > > -- > > Ververica GmbH | Invalidenstrasse 115, 10115 Berlin, Germany > > -- > Ververica GmbH > Registered at Amtsgericht Charlottenburg: HRB 158244 B > Managing Directors: Yip Park Tung Jason, Jinwei (Kevin) Zhang, Karl Anton > Wehner >
