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

Move the plugin to the FormattingService & ImportOptimizer APIs #1078

Merged
merged 16 commits into from
Apr 26, 2024

Conversation

crogoz
Copy link
Contributor

@crogoz crogoz commented Apr 26, 2024

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 and ImportOptimizer apis rather than doing the replacement of CodeStyleManager that we used to do.

Fixes #1068.

Possible downsides?

Project project = file.getProject();
PalantirJavaFormatSettings settings = PalantirJavaFormatSettings.getInstance(project);
Optional<FormatterService> formatter = formatterProvider.get(project, settings);
return Set.of(new PalantirJavaFormatImportOptimizer(formatter));
Copy link
Contributor Author

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) {
Copy link
Contributor Author

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());
Copy link
Contributor Author

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(
Copy link
Contributor Author

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.

@@ -23,7 +23,7 @@ def name = "palantir-java-format"
intellij {
pluginName = name
updateSinceUntilBuild = true
version = "IU-202.6397.94"
version = "2024.1"
Copy link
Contributor

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.

Copy link
Contributor Author

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

Copy link
Contributor

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?)

Copy link
Contributor Author

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)

Copy link
Contributor

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...

Copy link
Contributor

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

@CRogers CRogers marked this pull request as ready for review April 26, 2024 15:19
@CRogers
Copy link
Contributor

CRogers commented Apr 26, 2024

changelog bot?

@CRogers
Copy link
Contributor

CRogers commented Apr 26, 2024

👍

1 similar comment
@crogoz
Copy link
Contributor Author

crogoz commented Apr 26, 2024

👍

@CRogers
Copy link
Contributor

CRogers commented Apr 26, 2024

👍

1 similar comment
@felixdesouza
Copy link

👍

@bulldozer-bot bulldozer-bot bot merged commit 834c6c0 into develop Apr 26, 2024
7 checks passed
@bulldozer-bot bulldozer-bot bot deleted the callumr/fix-for-2024.1 branch April 26, 2024 16:02
@svc-autorelease
Copy link
Collaborator

Released 2.44.0

@UpperLevel-zz
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Formatting shortcut not intercepted in IntelliJ 2024.1
6 participants