palantir-java-format IntelliJ plugin ignores @Formatter:off/on

[SebHo0]

What happened?

The IntelliJ plugin ignores formatter guards (e.g. @Formatter:off/on) even though IntelliJ itself is setup to respect it.

What did you want to happen?

Do not overwrite the formatter guard settings (or at least provide a settings option to respect those).

[obilliard]

Any plan on fixing this? This indeed breaks the expected IntelliJ setup, as well as the potential Spotless config when combined.

[iamdanfox]

One of the guiding principles of this project was to minimize configurability in order to avoid endless discussions/arguments between developers about 'which formatting looks better'. I would be concerned that building support for special @formatter:off / on comments would allow/encourage folks to demarcate sections of their code where they want to use their own artisanal formatting, which re-opens the door to endless subjective discussions about what looks better.

So consequently I'd actually vote to not implement support for these (unless there's a super compelling use-case I'm not aware of). I think this is consistent with the google-java-format approach (https://github.com/diffplug/spotless/issues/275), gofmt. rustfmt does actually support these kinds of comments though (rustfmt_skip).

Are there snippets or examples where you feel it's essential to use the // @formatter:off comments?

[querdenker2k]

I need this as well. I am having code like this

new ResponseVerifier.ProductionRecordBuilder()
            .verifyArtifact("MyArtefact")
            .verifyDataFields()
            .verifyDataField("DataField1").mustExist().back()
            .verifyDataField("DataField2").mustNotExist().back()
            ...

The palantir plugin will currently put this all in single lines so this is not readable anymore. Why not taking respect to the IntellJ formatting guards. I can use the turn formatting-off/on feature with the maven-plugin as well.

[iamdanfox]

@querdenker2k I appreciate the snippet of code you've pasted is arguably better than the p-j-f formatted version:

new ResponseVerifier.ProductionRecordBuilder()
                .verifyArtifact("MyArtefact")
                .verifyDataFields()
                .verifyDataField("DataField1")
                .mustExist()
                .back()
                .verifyDataField("DataField2")
                .mustNotExist()
                .back()

but I'm afraid I don't find this example compelling enough to open the door to // @formatter:off. My concern is that adding this feature opens the door to people arguing about style within those blocks. For example, if you received a pull request with the snippet I pasted above, would you ask the contributor to use the // @formatter:off blocks and chain up the calls like .verifyDataField("DataField2").mustNotExist().back()?

We found somewhat similar cases internally, which led to us calling out the 'downside' of strongly opinionated formatters on the README:

if you don't like how the formatter laid out your code, you may need to introduce new functions/variables

In this case, I'd suggest maybe the verifyDataField() function could take two arguments - a string name and a lambda, within which you could configure the criteria. Then it might look something like the following snippet which palantir-java-format will automatically format:

new ResponseVerifier.ProductionRecordBuilder()
                .verifyArtifact("MyArtefact")
                .verifyDataFields()
                .verifyDataField("DataField1", field -> {
                    field.mustExist();
                })
                .verifyDataField("DataField2", field -> {
                    field.mustNotExist();
                })

[lazystone]

One of the arguments to support // @formatter:off could be to make a temporary workarounds for cases like #782

[jamesdh]

I think if a team is aware enough to be using a tool such as PJF, then when they see a code block that has formatting disabled via // @formatter:off then they'll also be aware enough to understand when it's helpful and permitted, and not worth arguing about the semantics of the chosen format. Anytime that's not the case, then the solution is likely to remove the format marker, but let the team decide for themselves.

[jamesdh]

The bigger concern is that this plugin breaks from IntelliJ default behavior, without any option to restore it or even a hint that it does so. The implication is that it modifies how IntelliJ does its formatting, when in fact it seems to replace the formatting function in its entirety.

[jamesdh]

FWIW, https://github.com/diffplug/spotless/issues/275 was also referenced along with the Google Java Formatter earlier as partial justification for not supporting this, but Spotless actually added the ability to disable formatting regardless of the fact GJF doesn't support it.