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

Modify IDE Hook to work with configuration cache #2278

Closed
wants to merge 10 commits into from

Conversation

lehmanju
Copy link

@lehmanju lehmanju commented Sep 26, 2024

This changes the implementation of the IDE hook to work with Gradle's configuration cache. Essentially, the old IDE hook implementation is deleted and the execution of the regular spotless task is augmented to filter the files before applying changes. If the property is missing, it just works like before.

As stdIn/stdOut properties are missing, I don't expect this to be merged in its current state, but I'd like to hear some feedback whether the approach is feasible. Ideally, I would also integrate #1101.

fixes #1082

@lehmanju
Copy link
Author

I changed the behavior to enable/disable the task based on whether a file is in the target set. This works quite well and avoids task execution for all other projects.

@nedtwigg
Copy link
Member

This seems like a great approach if you can get it to work!

@nedtwigg
Copy link
Member

Oh! Sorry, I spoke a bit too soon. This is going to interact a bit with the linting changes...

I'll try to get this ironed out this weekend...

@lehmanju
Copy link
Author

lehmanju commented Oct 3, 2024

This seems like a great approach if you can get it to work!

Yeah, the only thing that isn't working right now is the IDEHook test.

Tasks with empty target are disabled, which leaves only one task with exactly one file. Reexecuting on the same file successfully uses the configuration cache.

I plan to look into supporting multiple files at once. Maybe this can be integrated into IntelliJ IDEA to format all files before commiting.

@lehmanju
Copy link
Author

@nedtwigg I think it would make sense to move the functionality of -PspotlessIdeHookUseStdIn and -PspotlessIdeHookUseStdOut and IS DIRTY/IS CLEAN to a new task or remove it all together and break backwards compatibility.

In my understanding, only two projects use the IDE_HOOK functionality (Intellij and VSCode). The Intellij plugin doesn't care about any output or stdin/stdout functionality. The VSCode plugin only uses the IDE_HOOK with stdout/stdin and not by applying spotless to files directly. Also, it hasn't seen any activity in 2 years and might not be maintained anymore.

That's why I personally would simply drop that additional functionality and reimplement it, if it's needed later on. That would mean only "-PspotlessIdeHook" would remain.

@nedtwigg
Copy link
Member

The VSCode plugin only uses the IDE_HOOK with stdout/stdin and not by applying spotless to files directly. Also, it hasn't seen any activity in 2 years and might not be maintained anymore.

We've taken great pains to not change anything that would require an update!

I'd be okay breaking something if we have to, but I think we don't.

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.

spotlessIdeHook and configuration cache
2 participants