Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

configure code format for nbm plugin like maven #121

Closed
wants to merge 5 commits into from

Conversation

ebarboni
Copy link
Contributor

@ebarboni ebarboni commented Nov 9, 2023

Maybe more readable at the end. I hate curly bracket at the line :p

mvn spotless:apply to format if someone want.

Is this the intended use for it @cstamas ?

@ebarboni ebarboni requested a review from mbien November 9, 2023 12:25
@cstamas
Copy link
Member

cstamas commented Nov 9, 2023

Yes, but several remarks:

  • ignore blame rev will not work like this (basically you need to merge to get the "real" hash, and have a subsequent commit to add the real hash)
  • spotless has issue: it either works on java8 (but then does not work on Java21) or other way around 😄 Here you managed to fail it on both (8 and 21), most probable reason: you use spotless maven plugin that is Java11+ (but did not upgrade palantir itself) see https://github.com/maveniverse/parent/blob/main/pom.xml#L485-L487

@cstamas
Copy link
Member

cstamas commented Nov 9, 2023

Oh, re ignore blame rev: we use "squash merge", so new hash, but i think you do not use that... so I may be wrong here

@cstamas
Copy link
Member

cstamas commented Nov 9, 2023

Also, if you use maven-release-plugin for releasing, this is must (see related issue):
https://github.com/maveniverse/parent/blob/main/pom.xml#L493-L494

@mbien
Copy link
Member

mbien commented Nov 10, 2023

this looks unnecessary aggressive to me since it does far more than just fixing brace positioning.

Please keep the pom formatting since 4 space indent is also the NB default. I wouldn't like to see this merged in the current form tbh, the git blame commit ignore trick does only work for simple cases.

Comment on lines +217 to +228
<java>
<importOrder>
<file>config/maven-eclipse-importorder.txt</file>
</importOrder>
<removeUnusedImports />
<palantirJavaFormat>
<version>2.38.0</version>
</palantirJavaFormat>
<licenseHeader>
<file>config/maven-header-plain.txt</file>
</licenseHeader>
</java>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? This will conflict with NB settings right away.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you'are right.

@mbien
Copy link
Member

mbien commented Nov 10, 2023

I played a bit around with the NB formatter and this less-invasive setting gave better results:

<?xml version="1.0" encoding="UTF-8"?>
<project-shared-configuration>
    <properties xmlns="http://www.netbeans.org/ns/maven-properties-data/1">
        <org-netbeans-modules-editor-indent.text.x-toml.CodeStyle.project.indent-shift-width>2</org-netbeans-modules-editor-indent.text.x-toml.CodeStyle.project.indent-shift-width>
        <org-netbeans-modules-editor-indent.text.x-toml.CodeStyle.project.spaces-per-tab>2</org-netbeans-modules-editor-indent.text.x-toml.CodeStyle.project.spaces-per-tab>
        <org-netbeans-modules-editor-indent.text.x-go.CodeStyle.project.indent-shift-width>4</org-netbeans-modules-editor-indent.text.x-go.CodeStyle.project.indent-shift-width>
        <org-netbeans-modules-editor-indent.text.x-go.CodeStyle.project.tab-size>4</org-netbeans-modules-editor-indent.text.x-go.CodeStyle.project.tab-size>
        <org-netbeans-modules-editor-indent.text.x-go.CodeStyle.project.expand-tabs>false</org-netbeans-modules-editor-indent.text.x-go.CodeStyle.project.expand-tabs>
        <org-netbeans-modules-editor-indent.text.x-antlr3.CodeStyle.project.indent-shift-width>4</org-netbeans-modules-editor-indent.text.x-antlr3.CodeStyle.project.indent-shift-width>
        <org-netbeans-modules-editor-indent.text.x-yaml.CodeStyle.project.indent-shift-width>2</org-netbeans-modules-editor-indent.text.x-yaml.CodeStyle.project.indent-shift-width>
        <org-netbeans-modules-editor-indent.text.x-antlr4.CodeStyle.project.indent-shift-width>4</org-netbeans-modules-editor-indent.text.x-antlr4.CodeStyle.project.indent-shift-width>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.text-line-wrap>none</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.text-line-wrap>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.importGroupsOrder>*;static *</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.importGroupsOrder>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.indent-shift-width>4</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.indent-shift-width>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.separateStaticImports>true</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.separateStaticImports>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.spaces-per-tab>4</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.spaces-per-tab>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.enable-indent>true</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.enable-indent>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.enableCommentFormatting>false</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.enableCommentFormatting>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.tab-size>8</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.tab-size>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.text-limit-width>120</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.text-limit-width>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.expand-tabs>true</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.expand-tabs>
        <org-netbeans-modules-editor-indent.CodeStyle.project.text-line-wrap>none</org-netbeans-modules-editor-indent.CodeStyle.project.text-line-wrap>
        <org-netbeans-modules-editor-indent.CodeStyle.project.indent-shift-width>4</org-netbeans-modules-editor-indent.CodeStyle.project.indent-shift-width>
        <org-netbeans-modules-editor-indent.CodeStyle.project.spaces-per-tab>4</org-netbeans-modules-editor-indent.CodeStyle.project.spaces-per-tab>
        <org-netbeans-modules-editor-indent.CodeStyle.project.tab-size>8</org-netbeans-modules-editor-indent.CodeStyle.project.tab-size>
        <org-netbeans-modules-editor-indent.CodeStyle.project.text-limit-width>80</org-netbeans-modules-editor-indent.CodeStyle.project.text-limit-width>
        <org-netbeans-modules-editor-indent.CodeStyle.project.expand-tabs>true</org-netbeans-modules-editor-indent.CodeStyle.project.expand-tabs>
        <org-netbeans-modules-editor-indent.CodeStyle.usedProfile>project</org-netbeans-modules-editor-indent.CodeStyle.usedProfile>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.alignMultilineMethodParams>true</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.alignMultilineMethodParams>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.alignMultilineAnnotationArgs>true</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.alignMultilineAnnotationArgs>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.alignMultilineTryResources>true</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.alignMultilineTryResources>
    </properties>
</project-shared-configuration>

I would suggest to wait with this change. This would still need a quick manual scan-through with some reverts to filter out unnecessary changes which should be feasible since this codebase is relatively small.

Also @neilcsmith-net just fixed a bug in the formatter today.

@ebarboni
Copy link
Contributor Author

I played a bit around with the NB formatter and this less-invasive setting gave better results:

<?xml version="1.0" encoding="UTF-8"?>
<project-shared-configuration>
    <properties xmlns="http://www.netbeans.org/ns/maven-properties-data/1">
        <org-netbeans-modules-editor-indent.text.x-toml.CodeStyle.project.indent-shift-width>2</org-netbeans-modules-editor-indent.text.x-toml.CodeStyle.project.indent-shift-width>
        <org-netbeans-modules-editor-indent.text.x-toml.CodeStyle.project.spaces-per-tab>2</org-netbeans-modules-editor-indent.text.x-toml.CodeStyle.project.spaces-per-tab>
        <org-netbeans-modules-editor-indent.text.x-go.CodeStyle.project.indent-shift-width>4</org-netbeans-modules-editor-indent.text.x-go.CodeStyle.project.indent-shift-width>
        <org-netbeans-modules-editor-indent.text.x-go.CodeStyle.project.tab-size>4</org-netbeans-modules-editor-indent.text.x-go.CodeStyle.project.tab-size>
        <org-netbeans-modules-editor-indent.text.x-go.CodeStyle.project.expand-tabs>false</org-netbeans-modules-editor-indent.text.x-go.CodeStyle.project.expand-tabs>
        <org-netbeans-modules-editor-indent.text.x-antlr3.CodeStyle.project.indent-shift-width>4</org-netbeans-modules-editor-indent.text.x-antlr3.CodeStyle.project.indent-shift-width>
        <org-netbeans-modules-editor-indent.text.x-yaml.CodeStyle.project.indent-shift-width>2</org-netbeans-modules-editor-indent.text.x-yaml.CodeStyle.project.indent-shift-width>
        <org-netbeans-modules-editor-indent.text.x-antlr4.CodeStyle.project.indent-shift-width>4</org-netbeans-modules-editor-indent.text.x-antlr4.CodeStyle.project.indent-shift-width>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.text-line-wrap>none</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.text-line-wrap>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.importGroupsOrder>*;static *</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.importGroupsOrder>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.indent-shift-width>4</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.indent-shift-width>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.separateStaticImports>true</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.separateStaticImports>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.spaces-per-tab>4</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.spaces-per-tab>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.enable-indent>true</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.enable-indent>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.enableCommentFormatting>false</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.enableCommentFormatting>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.tab-size>8</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.tab-size>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.text-limit-width>120</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.text-limit-width>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.expand-tabs>true</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.expand-tabs>
        <org-netbeans-modules-editor-indent.CodeStyle.project.text-line-wrap>none</org-netbeans-modules-editor-indent.CodeStyle.project.text-line-wrap>
        <org-netbeans-modules-editor-indent.CodeStyle.project.indent-shift-width>4</org-netbeans-modules-editor-indent.CodeStyle.project.indent-shift-width>
        <org-netbeans-modules-editor-indent.CodeStyle.project.spaces-per-tab>4</org-netbeans-modules-editor-indent.CodeStyle.project.spaces-per-tab>
        <org-netbeans-modules-editor-indent.CodeStyle.project.tab-size>8</org-netbeans-modules-editor-indent.CodeStyle.project.tab-size>
        <org-netbeans-modules-editor-indent.CodeStyle.project.text-limit-width>80</org-netbeans-modules-editor-indent.CodeStyle.project.text-limit-width>
        <org-netbeans-modules-editor-indent.CodeStyle.project.expand-tabs>true</org-netbeans-modules-editor-indent.CodeStyle.project.expand-tabs>
        <org-netbeans-modules-editor-indent.CodeStyle.usedProfile>project</org-netbeans-modules-editor-indent.CodeStyle.usedProfile>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.alignMultilineMethodParams>true</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.alignMultilineMethodParams>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.alignMultilineAnnotationArgs>true</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.alignMultilineAnnotationArgs>
        <org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.alignMultilineTryResources>true</org-netbeans-modules-editor-indent.text.x-java.CodeStyle.project.alignMultilineTryResources>
    </properties>
</project-shared-configuration>

I would suggest to wait with this change. This would still need a quick manual scan-through with some reverts to filter out unnecessary changes which should be feasible since this codebase is relatively small.

Also @neilcsmith-net just fixed a bug in the formatter today.

Removing all checkstyle constrains + doing ALT-SHIFT-F on all sources (test included) and making it a "commit" that we could ignore could also be enough (gitblame seems working ok on surefire not expert by the way)

This issue is that (I almost sure) before the ALT-SHIFT-F reformat using the style (at the time mavenized, curly bracket on new line, + space and parenthesis) but nowadays it's reformat standard NetBeans java.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants