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

Maven IDE hook #1782

Merged
merged 9 commits into from
Sep 28, 2023
Merged

Maven IDE hook #1782

merged 9 commits into from
Sep 28, 2023

Conversation

lipiridi
Copy link

Duplicate the IDE hook that is implemented for the Gradle plugin - #568
The main issue pertains to supporting Maven in an IntelliJ plugin - #200 (comment)

Currently, I utilize the spotlessFiles argument for Maven-based projects in my IntelliJ plugin to execute the apply task only for the currently opened file in the editor. However, an IDE hook that accepts an absolute path is safer (as it's not a pattern) and a little more efficient.

I have also updated the example of spotlessFiles usage in the README, as it cannot accept slashes in the pattern. Instead, it is necessary to provide the regex dot .

@nedtwigg
Copy link
Member

Thanks! In Gradle, the spotlessIdeHook provides output using stdout, stderr, and stdin, like so

Do you plan to implement that here too? Or is only disk I/O enough for your case? If disk I/O is enough then we should be able to use spotlessFiles without adding spotlessIdeHook...

@lipiridi
Copy link
Author

I think I have just understood why it was implemented via stdout in the Gradle plugin.

Retrieving the edited file from the stdout is much faster because we don't access the I/O file. Moreover, the file will be saved in the file system by the IDE, not directly by Spotless. Am I correct?

If yes, then I'll try to replicate this feature in Maven and label the PR as a work in progress.

Thanks)

@lipiridi lipiridi changed the title Maven IDE hook [WIP] Maven IDE hook Aug 17, 2023
@nedtwigg
Copy link
Member

If yes, then I'll try to replicate this feature in Maven and label the PR as a work in progress.

That would be great!

Retrieving the edited file from the stdout is much faster because we don't access the I/O file. Moreover, the file will be saved in the file system by the IDE, not directly by Spotless. Am I correct?

It's not about speed, it's about letting the user work without saving and clobbering changes on disk that perhaps they didn't want to save over.

  • user opens a file
  • user changes the file within the editor
  • user asks to format this contents of the editor
  • Spotless returns the formatted contents of that editor to the editor

The point of the IDE hook is to facilitate that process without saving to disk in between the various steps. Saving to disk is fine for an IDE plugin, but you can use spotlessFiles for that, you don't need spotlessIdeHook.

@lipiridi lipiridi changed the title [WIP] Maven IDE hook Maven IDE hook Aug 24, 2023
@lipiridi
Copy link
Author

Hi @nedtwigg!
I've updated the PR and added the same IDE hook. Here's what's new:

  1. I removed the check ratchet.isClean() from the hook because the Maven plugin already collects only dirty files from Git.
  2. Unfortunately, I couldn't replicate the "diverge" test. I'm not sure how to do it via Maven. Any guidance on this would be appreciated.
  3. I reverted the README. Perhaps it's worth considering combining the IDE_HOOK.md for both plugins, but I'll leave that decision up to you.

@nedtwigg nedtwigg enabled auto-merge September 28, 2023 21:35
@nedtwigg nedtwigg merged commit 5d9f3b4 into diffplug:main Sep 28, 2023
11 checks passed
@nedtwigg
Copy link
Member

Finally released in plugin-maben 2.40.0. Terribly sorry that it took so long, this is a great PR for an important feature, so I wanted to do some testing before we released it. I just don't have much time at the moment for Spotless, so I released it with the "beta" warning in the changelog.

Once there is an IDE plugin working with the maven hook let me know, and we can update the docs to match and remove the beta.

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.

2 participants