Feature: -PspotlessIdeHook with multiple files

[aloifolia]

I am using -PspotlessIdeHook to have spotless format all files that lint-staged detects. So, before a commit succeeds, lint-staged triggers spotless individually for each file since the -PspotlessIdeHook parameter accepts single files only. For interested parties - this is my lint-staged configuration:

module.exports = {
    '*.{js,scss,md,ts,json,yml,yaml,html}': (filenames) =>
       filenames.map((filename) => `./gradlew spotlessApply "-PspotlessIdeHook=${filename}"`),
};

Although this approach works, the duration of which is quite underwhelming as gradle needs to start all-over for each file. It would be nice to be able to pass entire batches of file names to spotless (alternatively make lint-staged obsolete).

[nedtwigg]

The IDE hook can also read/write from stdin and/or stdout, which is useful for IDE integration. It is difficult to reconcile those features with multiple files, so I will not merge any PR which complicates the spotlessIdeHook task.

I am open to a new SpotlessApplyMulti.java, which could copy most of its logic from IdeHook. It could register a new spotlessApplyMulti task which reads a JSON list of absolute paths on stdin, and then applies to only those files. I would be okay with merging a PR for this if the logic exists almost exclusively inside of SpotlessApplyMulti.java.

broader thoughts on the usecase

When you stage a file, the content on disk can be different than the content in staging. Spotless only runs against the content on disk.

Which of the following is lint-staged doing?

• for each staged file
  • if the content on disk is the same as the content in staging
    • run spotlessApply on disk, and then put that into staged (safe)
  • else, disk is different than staging
    • (throw error?)
    • or run spotlessApply on disk, and clobber that into staged (dangerous)

In #623, we have a feature request which runs Spotless directly on staging, without touching disk. That has the benefit that when disk and staging are different, we never clobber that difference.

Advanced git users understand that there are at three complete copies (head, WC, staging/index), but most think of staging as just a boolean checkbox. This confusion makes it hard to build tools that do "the right thing" - either you clobber data for advanced users, or confuse everybody else by having the WC magically different than staging.

[aloifolia]

Concerning lint-staged, there is a blog which explains how lint-staged supports partially staged files. So (I have not tested it thourougly, yet), lint-staged seems to work this way:

• stash the working copy => now the staged version is on disk
• apply the registered tasks to those staged files that match some user-defined pattern (the tasks do not need to care about the staging vs. working copy problem)
• if all tasks succeed, add the new changes to the index (stage them) - this seems to be a bit more complicated
• if any error occurs, drop all new changes
• finalize the commit
• pop the stash => the working copy is on disk again

Anyhow, if spotless wants to eventually include the functionality that lint-staged currently covers, it would probably be wise to use/adapt that algorithm.

[nedtwigg]

Roger, based on how you described it above, this works the same way as #623, except that there's a (probably-unnoticeable) performance penalty from the stash/index->disk/unstash roundtrip.

Happy to merge SpotlessApplyMulti.java or an implementation of #623.

[headsvk]

I followed this blogpost about using spotless for pre-commit hooks. It works but it uses spotlessIdeHook per staged file which can be slow if you are committing many files. It would be great to be able to specify multiple files at once for this use case. This doesn't use lint-staged, first time I hear about that.

[ragurney]

This feature would actually really help with the format on save feature we're trying to add (ragurney/spotless-intellij-gradle#33) to the spotless-intellij-gradle plugin.

[xenoterracide]

following, because I too was thinking about whether I could use lintstaged (but with yaml, not js) with this. I can do it with a precommit hook before lintstaged, but then I can't only run it when certain files change. However right now that seems like the best option

[ragurney]

@nedtwigg just curious what the priority of this feature request is? It would be really great to have a separate -PspotlessIdeHookMulti hook or similar to support multiple files. If it's not prioritized to be worked on by the core team, I can try to take a swing at it when I have some free time.

[nedtwigg]

My priority list right now is

• GIT_ATTRIBUTES massive slowdown in Spotless 6.6.0+ #1527
• obsolete - Internal-only breaking changes #1096
• obsolete - Add support for linters #1097
• this

It is very useful to know that you want spotlessIdeHookMulti. I'll try to give an update here in ~2 weeks.

[ragurney]

Thanks so much @nedtwigg!

[ragurney]

Hey @nedtwigg, just following up here. Thanks!

[nedtwigg]

Sorry, going to be a couple weeks longer before I get to this, thanks for following up!

[ImUrX]

@nedtwigg bump >~<

[rlnt]

@nedtwigg 🙏🏼

[darioseidl]

I wanted to set up spotless on a pre-commit hook with husky and lint-staged. It works, but it's really slow when committing multiple files. Would be great to a have an option to pass multiple files to spotless. (Alternatively, if spotless could do what lint-staged does, #623 that would be nice too).

[nedtwigg]

• I'm not planning on building this myself, but now that Make IDE hook work with configuration cache, take 2 #2308 has merged, this should be pretty easy to add. PR's welcome.
• I lean towards making spotlessIdeHook support multiple files, comma-delimited, but a separate spotlessIdeHookMulti is okay too, implementor's choice