Add comptibility for 2024.1 IDEs #3569
Conversation
./gradlew wrapper --gradle-version=7.6.4 To fix gradle issue: gradle/gradle#27156
…, it was removed in 2024.1. (Note, this is only used in the jps-build test suite). This also removes references to Trove THashSet, and no longer stores File directly in sets or collections. See JetBrains/intellij-community@560e8fc
…mpatibility with IDEs v2024.1
|
Thanks for doing this. Would it be helpful if we try out the plugin? If you attach the plugin I'd be happy to install it and give it a go, so we can help boost the confidence that the plugin works as advertised? |
|
@gaggle Yes, I had the same thought. I'll finish my current round of testing, and later today I'll attach the .zip for users to try. |
|
hi @ashleysommer having an issue with the plugin where some coloring seems off and when i tried adding a moduledoc, the |
Yeah, there's probably a bunch of things still not working correctly. I'm actually very new to Elixir, I'm still learning what the features are and I was only using this plugin for 1 week before Jetbrains 2024.1 IDEs were released, and I'm still learning what this plugin is even capable of or what it should do correctly. |
|
It LGTM too! |
| @@ -82,7 +82,7 @@ allprojects { | |||
| } | |||
|
|
|||
| runPluginVerifier { | |||
| ideVersions = ["2023.3"] | |||
There was a problem hiding this comment.
Based on previous compatibility PRs we normally drop support for the previous version, so this should probably just be ["2024.1"]
There was a problem hiding this comment.
The old version is only dropped if a required code change makes it impossible to build for both versions. It's a coincidence that that has been the case for recent one.
There was a problem hiding this comment.
Gotcha, thanks for the clarification!
There was a problem hiding this comment.
The commit I used for reference was this one: 876b770
That does simply add the new version to the list, as I've done here.
| @@ -70,7 +70,7 @@ allprojects { | |||
| pluginDescription.set(bodyInnerHTML("resources/META-INF/description.html")) | |||
|
|
|||
| sinceBuild = "233.11799.241" | |||
| untilBuild = "233.*" | |||
| untilBuild = "241.*" | |||
There was a problem hiding this comment.
Based on previous compatibility PRs sinceBuild typically matches untilBuild since we're dropping support for previous versions.
There was a problem hiding this comment.
I haven't seen any compatibility PRs in this repo where the sinceBuild matches the untilBuild, that doesn't seem right.
There was a problem hiding this comment.
The following warning shows for this:
> Task :verifyPluginConfiguration
[gradle-intellij-plugin :verifyPluginConfiguration] The following plugin configuration issues were found:
- The 'since-build' property is lower than the target IntelliJ Platform major version: 233.11799.241 < 241.
I believe it needs to match.
Since we also only support the last version of the IDE per version, I think this is fine?
.github/workflows/test.yml
Outdated
| @@ -8,7 +8,7 @@ jobs: | |||
|
|
|||
| strategy: | |||
| matrix: | |||
| ideaVersion: [ "2023.3" ] | |||
| ideaVersion: [ "2023.3", "2024.1" ] | |||
There was a problem hiding this comment.
Pretty sure this can just be ["2024.1"] since we normally just support the current version.
|
|
||
| @Override | ||
| public void logDeletedFiles(Collection<String> paths) { | ||
| for (String path:paths){ | ||
| myDeletedFiles.add(new File(path)); | ||
| myDeletedFiles.add(new File(path).getAbsolutePath()); |
There was a problem hiding this comment.
Is this a change in behavior? I'm not sure if path was previously absolute or relative (it looks relative).
There was a problem hiding this comment.
The change is the set is storing the string version of the file instead of the File object. In the previous version, the handler was calling .getAbsolutePath() on every File object in the set, so we simply do that here instead and save a step.
| private fun usages(): String { | ||
| val ideVersion = Version.parseVersion(ApplicationInfoEx.getInstance().fullVersion) | ||
| val usagesString = if (ideVersion === null || ideVersion.lessThan(2021,2,0)) { | ||
| "Found usages" | ||
| } else { | ||
| } else if (ideVersion.lessThan(2024,1,0)) { | ||
| "Usages in" | ||
| } else { | ||
| "Usages" | ||
| } | ||
| return usagesString | ||
| } |
There was a problem hiding this comment.
I'm not sure I understand what's driving this change. Can you walk me through it please?
There was a problem hiding this comment.
This is to help some tests pass.
In February, IntelliJ changed the wording of the text representation of the find usages response. That change made its way into all 2024.1 IDEs and broke these tests.
JetBrains/intellij-community@0706bea
Note, there are still some other changes in 2024.1 that are preventing these tests from actually passing, but this is the simple first step to remedy the bulk of the error messages.
There was a problem hiding this comment.
Is it necessary to support previous build versions if we are bocking these versions with the from-version attribute?
There was a problem hiding this comment.
Good point, we really don't need the case to handle the pre-2021.2 usages string.
It had been in the plugin code right up to and including the 2023.3 release, thats why I left it in there in this change, but refactoring it like this could be a good opportunity to remove that old version compatibility.
|
Thanks for looking at this! Note that while it does allow the plugin to load I am seeing various UI issues with intellij-elixir-17.0.0-pre+20240408035552.zip. Most annoyingly I have lost the matching brackets highlight - you don't realise how much you use that until it is gone! Also comments are showing as black, the same as normal code, which is getting very confusing.
Is anyone seeing either of those issues? Edit - also we seem to have lost the ability to run tests via the IDE |
|
This worked fine for me with just updating until-version. I also played with changing org.jetbrains. I did need to purge m2 deps however. #3579 |
|
@ashleysommer @KronicDeth I have a commit in my fork that I believe fixes the failing I could use some feedback on this; I think the difference is just formatting for the usage output, in which case this commit should be good. I'm also still seeing the following test failures: These pass for me locally on the main branch and fail on @ashleysommer's branch, so I don't believe it is simply my environment setup. |
Yes, that is the main remaining test failure that I've been working to fix the last couple of weeks. I've been working on a way that can assert success of the tests regardless of the whitespace formatting in the returned usages serialization.
I don't see any of those |
I think I've narrowed down where this issue is coming from. I've found one difference in the parser in 2024.1 regarding brackets that must be causing this. |
Is this needed? From what I remember recent updates of this plugin required an upgrade of the IDE. |
I wouldn't know, I only started using this plugin in March 2024, after IntelliJ 2023.3 was released. Looking back through the upgrade commits, it seems like sometimes the min version is bumped, sometimes it isn't.
Well, no its not strictly needed, but all other changes in this branch have kept backwards compatibility with 2023.3 IDE, normally when I contribute to an open source project the expectation is to avoid breaking changes at all costs wherever possible. |
I have tested your build and it was not working with the newest IDEA, but it does work with the previous one making IEx and debugging work in the IDE with Elixir 1.16 and Erlang OTP 26. This is very helpful - thanks! Looking forward to seeing those in the next release. |
That binary you shared installs pretty well on my IntelliJ IDEA 2024.1.1 (Ultimate Edition) Build #IU-241.15989.150, built on April 29, 2024 |
|
Hey guys, is there any ETA for this? |
It certainly works fine for me, across both an Intel Mac and an ARM Mac. I have IntelliJ Ultimate 2024.1.1 installed, with |
well, it "works", but found that like the auto complete of say parentheses etc are gone and the colors come and go if the syntax is "broken" which happens as you type character by character. |
|
per formatting issues. branch by abstraction. Just clone the formatting logic, add a toggle to switch between the new/old logic. Make it work for just the new ide version then start comparing the change diffs to make the new iteration backwards compatible. |
|
If you can hack that to work properly I can help with the switching logic. |
|
I've been using Zed on MacOS for elixir. The inline Heex syntax highlighting has changed my life. I have nothing but love for all the work @KronicDeth put into this plugin but I can understand why maintaining a plugin for a profitable company is not that rewarding. Especially with breaking changes from upstream. |
|
I didn't want to go back to VSCode, nor leave Linux, but I see that the main tools come out first for Mac. Maybe I'll change my operating system since I'm diving into Elixir and don't want to spend time adjusting Neovim. |
You could try building from source! |
I too migrated to Zed for now. It's fast and just works. Can also keep pretty much the same key bindings. But I do miss a lot of features that Jetbrains IDEs offer |
Thanks for suggestion boss. Will be trying zed... |
|
Also #3362 (comment) should hopefully give you more understanding and empathy about opensource development. |
|
Thank you so much for this contribution, I will take a look through the code, the existing reviews and will try and merge in the next few days, if everything goes well. Please also see #3598 . |
There was a problem hiding this comment.
Awesome work so far, thanks so much.
For reference the following tests need to be fixed - I'll get these done.
/tests/org/elixir_lang/FindUsagesTest.kt#L161
There are some slight differences in indentation between the expected and actual results. Here's a diff to highlight the changes:
<root> (3)
Usages (3)
Call definition clause (2)
light_idea_test_case (2)
(2)
function_recursive_usage.ex (2)
- 2def function([], acc), do: acc
- 4def function([h | t], acc) do
+ 2 def function([], acc), do: acc
+ 4 def function([h | t], acc) do
Value read (1)
light_idea_test_case (1)
(1)
function_recursive_usage.ex (1)
- 5function(t, [h | acc])
+ 5 function(t, [h | acc])The main differences are:
- Additional indentation (2 spaces) before
defon lines 2 and 4 - Additional indentation (4 spaces) before
functionon line 5
23 failing tests, but if we fix the root cause, they should pass.
And:
/tests/org/elixir_lang/code_insight/highlighting/brace_matcher/Issue443.java#L17
it tests this file:
/testData/org/elixir_lang/code_insight/highlighting/brace_matcher/issue_443/do_block.ex#L1
error is:
`do` not matched to `end`
I believe 2024.1 changed how the Usages work.
In 2023.x:
```kotlin
val usages =
myFixture.testFindUsagesUsingAction("module_attribute_usage.ex",
"kernel.ex")
.map { it as UsageInfo2UsageAdapter }
```
In the debugger shows:
```
0 = {ReadWriteAccessUsageInfo2UsageAdapter@19239} "4|def usage, |do|:
|@|module_attribute"
1 = {ReadWriteAccessUsageInfo2UsageAdapter@19240} "2|@|module_attribute|
|1"
```
For 2024.1, this shows:
```
0 = {ReadWriteAccessUsageInfo2UsageAdapter@19421} "2|
|@module_attribute| 1"
1 = {ReadWriteAccessUsageInfo2UsageAdapter@19422} "4| def usage, do:
|@module_attribute"
```
I believe it not shows the whitespace for the file, where previously it
didn't.
|
Another update/devlog.. I should move this somewhere else, probably. As noted in the Proposed Roadmap, after this task we'll try and figure out how to reduce friction and "modernise" the process - a lot has changed in the last few years with both the Java/Kotlin and the IDEA Plugin Platform ecosystem. Right now we are down to a few errors when testing.
This seems to return
Configuration file for j.u.l.LogManager does not exist: /home/josh/.gradle/caches/modules-2/files-2.1/com.jetbrains.intellij.idea/ideaIC/2024.1/181fa36f74690e64a81a8e06ceda9480d2a6c626/ideaIC-2024.1/test-log.properties
Cannot invoke "com.intellij.openapi.application.Application.getService(java.lang.Class)" because the return value of "com.intellij.openapi.application.ApplicationManager.getApplication()" is null
java.lang.NullPointerException: Cannot invoke "com.intellij.openapi.application.Application.getService(java.lang.Class)" because the return value of "com.intellij.openapi.application.ApplicationManager.getApplication()" is null
at com.intellij.notification.NotificationGroupManager.getInstance(NotificationGroupManager.java:19)
at org.elixir_lang.jps.HomePath.eachEbinPath(HomePath.java:47)
at org.elixir_lang.jps.BuilderTest.addErlangSdk(BuilderTest.java:143)
at org.elixir_lang.jps.BuilderTest.setUp(BuilderTest.java:172)
at com.intellij.testFramework.UsefulTestCase.invokeSetUp(UsefulTestCase.java:430)
at com.intellij.testFramework.UsefulTestCase.defaultRunBare(UsefulTestCase.java:422)
at com.intellij.testFramework.UsefulTestCase.lambda$runBare$12(UsefulTestCase.java:491)
at com.intellij.testFramework.EdtTestUtil.lambda$runInEdtAndWait$3(EdtTestUtil.java:80)
at com.intellij.openapi.application.impl.RwLockHolder.runWriteIntentReadAction(RwLockHolder.kt:70)
at com.intellij.testFramework.EdtTestUtil.lambda$runInEdtAndWait$4(EdtTestUtil.java:79)
at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:308)
at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:771)
at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:722)
at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:716)
at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:86)
at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:741)
at com.intellij.ide.IdeEventQueue.dispatchEvent(IdeEventQueue.kt:326)
at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)
Cannot invoke "com.intellij.openapi.application.Application.getService(java.lang.Class)" because the return value of "com.intellij.openapi.application.ApplicationManager.getApplication()" is null
java.lang.NullPointerException: Cannot invoke "com.intellij.openapi.application.Application.getService(java.lang.Class)" because the return value of "com.intellij.openapi.application.ApplicationManager.getApplication()" is null
at com.intellij.notification.NotificationGroupManager.getInstance(NotificationGroupManager.java:19)
at org.elixir_lang.jps.HomePath.eachEbinPath(HomePath.java:47)
at org.elixir_lang.jps.BuilderTest.addErlangSdk(BuilderTest.java:143)
at org.elixir_lang.jps.BuilderTest.setUp(BuilderTest.java:172)
at com.intellij.testFramework.UsefulTestCase.invokeSetUp(UsefulTestCase.java:430)
at com.intellij.testFramework.UsefulTestCase.defaultRunBare(UsefulTestCase.java:422)
at com.intellij.testFramework.UsefulTestCase.lambda$runBare$12(UsefulTestCase.java:491)
at com.intellij.testFramework.EdtTestUtil.lambda$runInEdtAndWait$3(EdtTestUtil.java:80)
at com.intellij.openapi.application.impl.RwLockHolder.runWriteIntentReadAction(RwLockHolder.kt:70)
at com.intellij.testFramework.EdtTestUtil.lambda$runInEdtAndWait$4(EdtTestUtil.java:79)
at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:308)
at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:771)
at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:722)
at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:716)
at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:86)
at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:741)
at com.intellij.ide.IdeEventQueue.dispatchEvent(IdeEventQueue.kt:326)
at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)Trawling through Google shows this issue arised about a year ago. This post has "answered in Slack"... but Slack doesn't keep history and the post is closed 🙄.
So we'll need to dig into why this is happening. |
|
Okay, figured it out, we needed to change Thanks for your work to get this started, I've added a few changes and it's green now, so I'm going to push this. |
|
Thanks so much! |
|
Awesome! Will there be a new release for IntelliJ 2024 users? |
|
Yes, there will be once I'm confident things work as expected. Right now I think there is an issue with upgrading and facets. I'm thinking of adding a new button for clearing facets, incase things "bug out", which is acceptable as you just need to readd the Erlang/Elixir SDKs. Please see #3604 The other problem IntelliJ now enforces a fast UI, which is great. But a lot of the code for the Elixir plugin needs to be ported. Until then, you can use You should be able to use this without much issues with that setting, we'll aim to get this sorted by moving it to background threads etc. |
* Update gradlewrapper to v7.6.4 ./gradlew wrapper --gradle-version=7.6.4 To fix gradle issue: gradle/gradle#27156 * Remove use of `FileUtil.FILE_HASHING_STRATEGY` from Intellij FileUtil, it was removed in 2024.1. (Note, this is only used in the jps-build test suite). This also removes references to Trove THashSet, and no longer stores File directly in sets or collections. See JetBrains/intellij-community@560e8fc * Add support for 2024.1 IDEs (and runs tests correctly against 2024.1) * Update usages group wording for 2024.1 * Fix more sdk configuration commits in application RW thread, fixes compatibility with IDEs v2024.1 * Fix whitespace in tests due to 2024.1 change I believe 2024.1 changed how the Usages work. In 2023.x: ```kotlin val usages = myFixture.testFindUsagesUsingAction("module_attribute_usage.ex", "kernel.ex") .map { it as UsageInfo2UsageAdapter } ``` In the debugger shows: ``` 0 = {ReadWriteAccessUsageInfo2UsageAdapter@19239} "4|def usage, |do|: |@|module_attribute" 1 = {ReadWriteAccessUsageInfo2UsageAdapter@19240} "2|@|module_attribute| |1" ``` For 2024.1, this shows: ``` 0 = {ReadWriteAccessUsageInfo2UsageAdapter@19421} "2| |@module_attribute| 1" 1 = {ReadWriteAccessUsageInfo2UsageAdapter@19422} "4| def usage, do: |@module_attribute" ``` I believe it not shows the whitespace for the file, where previously it didn't. * pin versions * use BasePlatformTestCase to stop warning * add do block match test * set 241.0 as the version, to fix certain intellij warnings * fix key * revert tests * use thread --------- Co-authored-by: Josh Taylor <joshuataylorx@gmail.com>
… Comment) (#3582) * feat: enabling proper code generation for comments (toggleable, Comment with Line/Block Comment) * Add the correct ERL and elixir arguments for starting IEx depending on the version of elixir sdk * Bundle latest OtpErlang.jar from JInterface v1.14 for OTP v26 * Infer OTP_RELEASE & ERLANG_SDK_HOME if no environment variable is set (#3600) * Add compatibility for 2024.1 IDEs (#3569) * Update gradlewrapper to v7.6.4 ./gradlew wrapper --gradle-version=7.6.4 To fix gradle issue: gradle/gradle#27156 * Remove use of `FileUtil.FILE_HASHING_STRATEGY` from Intellij FileUtil, it was removed in 2024.1. (Note, this is only used in the jps-build test suite). This also removes references to Trove THashSet, and no longer stores File directly in sets or collections. See JetBrains/intellij-community@560e8fc * Add support for 2024.1 IDEs (and runs tests correctly against 2024.1) * Update usages group wording for 2024.1 * Fix more sdk configuration commits in application RW thread, fixes compatibility with IDEs v2024.1 * Fix whitespace in tests due to 2024.1 change I believe 2024.1 changed how the Usages work. In 2023.x: ```kotlin val usages = myFixture.testFindUsagesUsingAction("module_attribute_usage.ex", "kernel.ex") .map { it as UsageInfo2UsageAdapter } ``` In the debugger shows: ``` 0 = {ReadWriteAccessUsageInfo2UsageAdapter@19239} "4|def usage, |do|: |@|module_attribute" 1 = {ReadWriteAccessUsageInfo2UsageAdapter@19240} "2|@|module_attribute| |1" ``` For 2024.1, this shows: ``` 0 = {ReadWriteAccessUsageInfo2UsageAdapter@19421} "2| |@module_attribute| 1" 1 = {ReadWriteAccessUsageInfo2UsageAdapter@19422} "4| def usage, do: |@module_attribute" ``` I believe it not shows the whitespace for the file, where previously it didn't. * pin versions * use BasePlatformTestCase to stop warning * add do block match test * set 241.0 as the version, to fix certain intellij warnings * fix key * revert tests * use thread --------- Co-authored-by: Josh Taylor <joshuataylorx@gmail.com> * Add 18.0.0 changelog, fix version (#3602) * support only the newest version. * remove tests for now, kind of pointless --------- Co-authored-by: Ashley Sommer <ashleysommer@gmail.com> Co-authored-by: Josh Taylor <joshuataylorx@gmail.com>
Alternative to #3567
There were a couple more changes required other than simply bumping the IntelliJ IDE version number.
When I try to build this plugin against 2024.1 IDEs and run the Test suite, I get the error
NoSuchFieldError: FILE_HASHING_STRATEGY. See relevant underlying IDE code changeWhile in the process of fixing that issue, I upgraded the
org.jetbrains.intellijgradle plugin tov1.17.3as recommended in the IntelliJ docs. Unfortunately that brings in some other gradle dependencies that require gradlev7.6.4minimum see here for details.So this PR bumps IDE version to "2024.1", adds that version to the test suite
PluginVerifiermatrix, fixes theNoSuchFieldErrorinjps-buildtests, bumpsorg.jetbrains.intellijgradle plugin tov1.17.3, and bumps gradlewrapper version tov7.6.4.