-
Notifications
You must be signed in to change notification settings - Fork 49
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
Move the plugin to the FormattingService & ImportOptimizer APIs #1078
Conversation
Project project = file.getProject(); | ||
PalantirJavaFormatSettings settings = PalantirJavaFormatSettings.getInstance(project); | ||
Optional<FormatterService> formatter = formatterProvider.get(project, settings); | ||
return Set.of(new PalantirJavaFormatImportOptimizer(formatter)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
different from google-java-format: we need to pass the formatter to the Optimizer class, such that we can call the spi implementation
} | ||
} | ||
|
||
public static String applyReplacements(String input, Collection<Replacement> replacementsCollection) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same implementation as in Utils.applyReplacements
@BeforeEach | ||
public void setUp() throws Exception { | ||
TestFixtureBuilder<IdeaProjectTestFixture> projectBuilder = IdeaTestFixtureFactory.getFixtureFactory() | ||
.createLightFixtureBuilder(getProjectDescriptor(), getClass().getName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using .createLightFixtureBuilder(getProjectDescriptor(), getClass().getName());
instead of .createFixtureBuilder(getClass().getName());
such that we can set up the sdk in: https://github.com/palantir/palantir-java-format/pull/1078/files#diff-bc3ac8002ebba51e0734b16f06fb1ebf8aadbe6c006c247cb80fcd72c877a2f5R88
same for the other test
|
||
@Test | ||
public void defaultFormatSettings() throws Exception { | ||
String input = Files.readString( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deviates from the google-java-format test. This test only checks the PalantirJavaFormatFormattingService
workflow.
palantir-java-format-spi/src/main/java/com/palantir/javaformat/java/FormatterService.java
Show resolved
Hide resolved
palantir-java-format-spi/src/main/java/com/palantir/javaformat/java/FormatterService.java
Outdated
Show resolved
Hide resolved
@@ -23,7 +23,7 @@ def name = "palantir-java-format" | |||
intellij { | |||
pluginName = name | |||
updateSinceUntilBuild = true | |||
version = "IU-202.6397.94" | |||
version = "2024.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will need to check if this is actually true, but I think we need to set this back down to a lower version before we merge - google-java-format had it at 2021.3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when running ./gradlew runIde
the intelij version is picked up from this version. Can leave it at 2021.3 though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs say:
The version of the IntelliJ Platform IDE that will be used to build the plugin.
I think this means the SDK, especially given the errors about LightProjectDescriptor
not existing in the tests. We should check whether this means you need at least this version of IntelliJ to run it (or perhaps it's fine provided you don't use "new" methods?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
especially given the errors about LightProjectDescriptor not existing in the tests.
yes, I think this is the version used for building & testing the plugin
least this version of IntelliJ to run it
I think 'since-build' does this. So we are saying that our plugin is compatible with at least 2021.3 (213)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think it's probably fine - the only risk is someone later tries to use some very new method. But I don't expect any work on this intellij plugin needed beyond bugfixes...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the plugin built using 2024.1 works fine with 2023.1.6
changelog bot? |
👍 |
1 similar comment
👍 |
👍 |
1 similar comment
👍 |
Released 2.44.0 |
I'm using version 2.46.0 and the problem still occurres. My intelliJ version is 2024.1.1. The buildnumber is #IU-241.15989.150. Anyone else has the same problem |
Before this PR
IntelliJ 2024.1 stops formatting via palantir-java-format when you press the format shortcut key or save (if formatting on actions on save is enabled).
After this PR
Similar to this google-java-format commit, we move to the new
FormattingService
andImportOptimizer
apis rather than doing the replacement ofCodeStyleManager
that we used to do.Fixes #1068.
Possible downsides?