spotless icon indicating copy to clipboard operation
spotless copied to clipboard

Compatibility considerations for the IDE hook.

Open nedtwigg opened this issue 2 years ago • 3 comments

The Spotless IDE hook should just be spotlessApply -PspotlessIdeHook=${ABSOLUTE_PATH_TO_FILE}. We've had this API since 3.30.0, and you shouldn't have to parse a Gradle or Spotless version to use it.

But we've got two backward incompatible problems:

  1. Spotless IDE hook is not compatible with the configuration-cache, so you have to specify --no-configuration-cache. But, that flag is not compatible with Gradle < 6.6. So you're stuck in this position where you have to parse the Gradle version to know whether you need that flag or not.

  2. We can speed Spotless up a lot by changing the API to :spotlessApply -PspotlessIdeHook=BLAH, but only after #1101 gets implemented. So you're stuck in this position where you have to parse the Spotless version to know whether you can add the leading colon or not.

We could fix both of those issues by changing the API to this:

:spotlessIdeHook -PspotlessIdeHook=BLAH --configure-on-demand

This fixes problem 1 because Gradle 7.4 adds a new Task.notCompatibleWithConfigurationCache() method. We can't use that method to fix problem 1 for spotlessApply, but we could use it to fix a new task called spotlessIdeHook.

It also fixes problem 2 because we can change the implementation of :spotlessIdeHook over time. At first it would trigger the evaluation of all subprojects (like spotlessApply does now), but later on it could take advantage of configure-on-demand to speed up execution dramatically.

So I have a question for each of you, @badsyntax and @ragurney, which is easier?

A) Parse Gradle and Spotless versions to determine which arguments to use. B) Optimistically try :spotlessIdeHook method, if it fails fall back to spotlessApply, and then remember "optimistic attempt failed" for the rest of the plugin's runtime. No parsing needed, just detecting an error.

As an aside, Spotless is adding support for linters in the medium-term future, and those lints will be available in the IDE hook. We are able to add this information in a backward-compatible way, so it will be optional whether you choose to add support for that in future, no version-parsing or feature-detection required.

nedtwigg avatar Jan 18 '22 20:01 nedtwigg

Hi @nedtwigg. I don't have enough knowledge of the IntelliJ SDK know off-hand which would be easier -- I'll have to do some investigation -- but overall I think I prefer option A. It seems like it would provide a better user experience.

ragurney avatar Jan 31 '22 18:01 ragurney

@ragurney added Gradle version parsing in https://github.com/ragurney/spotless-intellij-gradle/pull/22.

One option to allow the speedup mentioned in 2. above (switching from spotlessApply to :spotlessApply would be for the Spotless plugin to alter behavior based on Gradle version, since both Spotless and its users can easily determine the Gradle version, whereas its harder for users to determine the version of Spotless.

nedtwigg avatar Feb 12 '22 01:02 nedtwigg

@nedtwigg apologies for the belated response. I'm going with option A. I've started the process to make this happen in vscode (see https://github.com/microsoft/vscode-gradle/issues/1156) but haven't had time to chase it up. It's in the works though.

badsyntax avatar Feb 12 '22 08:02 badsyntax