-
Notifications
You must be signed in to change notification settings - Fork 101
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
IDEA 2022.x support #1097
IDEA 2022.x support #1097
Conversation
I recommend you to reconsider older IDEA versions support, as since 2022.* supporting older builds seems not possible. I may be wrong, but I leave this decision up to you for resolution. Adding version-specific patches to barely maintainable open-source project is not an easy engineering, and this may lead the plugin to eternally-stale state |
case "2020.2" : plugins.add('com.intellij.flex:202.6397.59'); break | ||
case "2020.3" : plugins.add('com.intellij.flex:203.5981.155'); break | ||
case "2021.1" : plugins.add('com.intellij.flex:211.6693.111'); break | ||
case "2022.3" : plugins.add('com.intellij.flex:223.7571.4'); break |
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.
tested specifically on 2022.3
, not compatible with 2022.3.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.
i think its about time we drop support for older versions if they are holding us back due to API changes etc. but perhaps a do a release for all older versions before we move on. it would be nice if we could follow the latest version so people can upgrade their IDEs
case "2020.3" : plugins.add('com.intellij.flex:203.5981.155'); break | ||
case "2021.1" : plugins.add('com.intellij.flex:211.6693.111'); break | ||
case "2022.3" : plugins.add('com.intellij.flex:223.7571.4'); break | ||
case "2022.3.3" : plugins.add('com.intellij.flex:223.8617.9'); break |
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.
tested specifically on 2022.3.3
, not compatible with 2022.3
@@ -320,13 +328,13 @@ task cleanGenerated(type: Delete, group: 'generate') { | |||
task generateHaxeParser(dependsOn: ':setupTools', type: JavaExec, group: 'generate') { | |||
workingDir = "${toolDir}" | |||
main '-jar' | |||
args = ['grammar-kit.jar', "${generatedSrcDir}", "${grammarHaxe}"] | |||
args = ['--add-opens=java.base/java.lang.reflect=ALL-UNNAMED', '--add-opens=java.base/java.util=ALL-UNNAMED', '--add-opens=java.base/java.lang=ALL-UNNAMED', 'grammar-kit.jar', "${generatedSrcDir}", "${grammarHaxe}"] |
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.
these and next jvmArgs are added as reflection is restricted on newer JREs
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.
these and next jvmArgs are added as reflection is restricted on newer JREs
@rosingrind beware that GrammarKit has a Gradle plugin now, exposing GenerateLexerTask
and GenerateParserTask
.
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.
totally understandable, however the next step you should consider to perform transition to newer versions - dropping gradle-grammar-kit-plugin
completely as it does not work as expected, see JetBrains/gradle-grammar-kit-plugin#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.
I left gradle-grammar-kit-plugin
configurations as-is because it haven't introduced new problems yet, but it may do so potentially. Correct implementation of utilizing gradle-grammar-kit-plugin
means double-pass generation as there are some clashing problems in gradle JVM and gk-plugin generation steps, see linked issue for more info
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.
@rosingrind interesting. I wasn't aware of this, and I see it's a long standing issue too. Good catch, thanks!
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.
i have wanted to upgrade grammar-kit and use the plugin but the reason why we stuck with the old version was because newer versions would output different code on different operating systems. but this could have been solved by now, so we could test the plugin and se how it goes
@@ -80,7 +80,7 @@ | |||
<depends optional="true" config-file="debugger-support.xml">com.intellij.modules.ultimate</depends> | |||
|
|||
<!-- Leave the '999' at the start of unreleased version numbers so that it's always newer than any released version. --> | |||
<version>999 Unreleased Post 1.3.1@plugin.dev.version@ for @plugin.compatibility.description@</version> | |||
<version>DEV @plugin.dev.version@ for @plugin.compatibility.description@</version> |
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.
cut as maximum allowed is 64 characters long
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.
We left the initial number high on purpose, and it got updated as a release step. The primary reason is so that IDEA doesn't go out and try to download the released version over the top of a custom version you're developing. YMMV.
The 64 char limit must be new.
@@ -68,7 +69,8 @@ public HaxePsiBuilder(@NotNull Project project, | |||
@NotNull Lexer lexer, | |||
@NotNull LighterLazyParseableNode chameleon, | |||
@NotNull CharSequence text) { | |||
super(project, parserDefinition, lexer, chameleon, text); | |||
// FIXME: https://github.com/JetBrains/intellij-community/commit/bf92a4bfea21ac1269017a52a347ab32c1a87323#r105729752 |
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.
super()
call should work unchanged, see commit discussion for clarification
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.
fixed in JetBrains/intellij-community@be794f3: wait for a release tag, bump plugin SDK and roll back (ASTNode)
cast
cool, thanks for putting inn all this work, ill try to take a closer look during the weekend :) |
@@ -36,7 +36,7 @@ | |||
<table width="100%" cellpadding="5px" border="0" style="background-color:rgb(168,75,56);"> | |||
<tr"> | |||
<td align="right"><b>Documentation:</b></td> | |||
<td align="left"><a style="color:white;" href="http://intellij-haxe.org">Project Web Site</a></td> | |||
<td align="left"><a style="color:white;" href="https://intellij-haxe.org">Project Web Site</a></td> |
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.
Note to self:
- add Discord info ?
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.
Just a note regarding https. The intellij-haxe web site does not have a cert, so can't be addressed using https at this time (you will get a "this web site is not secure" screen from your browser). I don't feel like paying $100/yr to the ISP just to protect a read-only web site that I'm not actively maintaining. If somebody knows how to set up a cert manually that can be deployed without paying the ISP (Ionos), I'm all ears.
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.
you could use letsencrypt + certbot if you control the system and can install software
(i personally use certbot + bypass as my provider as their certificate lasts longer before you have to renew)
@@ -46,12 +46,6 @@ public boolean isParsable(@NotNull CharSequence buffer, @NotNull Language fileLa | |||
return false; | |||
} | |||
|
|||
@Override | |||
public PsiBuilder parseLight(ASTNode chameleon) { |
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.
any reason why this has to be removed, did it break anything ?
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.
these methods does not exist anymore
@rosingrind I'd bump to 2023.1 ( |
I just have to say, this is really great you have put time into this @rosingrind. I think if this plugin can be maintained, it opens a lot more possibilities for Haxe in the future, for those that use IDEA everyday for food. Thanks again. |
@teotigraphix there are still a lot of improvements (2023.1 just got released), but at least I'm glad I am allowed to put a fresh start for this plugin. If this update happens to be deployed then following support will certainly be easier, and this makes me happy too. Obviously, for any maintainer this is not a small decision to accept such a pull, and I plan to continue on polishing and renovating (by coincidence, I'm interested in this) - so now we just relax and wait |
I was looking for this PR for ages! Thanks for the work you have put up for this. |
@alexk74 honestly, this is not enough info for me to pinpoint an issue, I'm assuming you didn't set up your dev environment correctly. I'll leave pre-built reference version here just in case (tested on 2022.3.3): intellij-haxe-2022.3.zip |
<tr> | ||
<td align="right"><b>Enterprise Support:</b></td> | ||
<td align="left"><a style="color:white;" href="http://bishtonsoftwaresolutions.com/">Bishton Software Solutions</a></td> | ||
<td align="left"><a style="color:white;" href="https://bishtonsoftwaresolutions.com/">Bishton Software Solutions</a></td> | ||
</tr> |
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.
We can drop this stanza as there are no customers for Enterprise Support at this time, and I'm not actively involved any more.
</tr> | ||
<tr> | ||
<td align="right"><b>Enterprise Support:</b></td> | ||
<td align="left"><a style="color:white;" href="http://bishtonsoftwaresolutions.com/">Bishton Software Solutions</a></td> | ||
<td align="left"><a style="color:white;" href="https://bishtonsoftwaresolutions.com/">Bishton Software Solutions</a></td> | ||
</tr> | ||
<tr> |
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 same is true for this table row (through line 60), since I no longer am accepting donations via Patreon.
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.
Also, the table at (62-66)
@rosingrind Thanks for the help and for the zip file, I really appreciate it. I managed to open the editor after I installed the plugin from you. When I built it myself, only intellij-haxe.jar was produced, I guess the dependencies should have been built too. I got a few errors though, but I can work with them. For the filter function it shows a type mismatch if argument n is typed.
When the editor starts, it throws this plugin exception:
Also, as a mention, when starting a new project from 2022.3 it does not show the option to create a Haxe project like the 2020 version does. |
That's a separate bug entirely, having to do with the internal (Haxe) type resolver. Please file it separately. Thanks. |
@alexk74
In the folder build/distributions is a zip-archive with a name like "intellij-haxe-", which contains the necessary dependencies and works correctly. |
Do you have plans to get rid of AnnotationHolder's implementation (implemented in HaxeAnnotationHolder) as it marked as @ApiStatus.NonExtendable? |
Summary
This pull request is made to introduce support for modern IDEA versions starting from 2022.3. Any changes introduced by merging this pull request may break builds for older versions - read next:
Dependencies
All plugin dependencies are upped according to bare minimum 2022.3 support. It's possible to build on any
223.*
platform version,2022.3
and2022.3.3
tested (entry forflex
plugin left questionable as currentversionWithMajor
pattern does not support patch number)log4j
replaced withreload4j
as advised by: JetBrains/intellij-deps-log4j@3de83be: https://blog.jetbrains.com/platform/2022/02/removing-log4j-from-the-intellij-platform/java
platform bumped to 17: https://blog.jetbrains.com/platform/2022/08/intellij-project-migrates-to-java-17org.jetrbrains.intellij
bumped to 1.10.1 following migration guide: https://lp.jetbrains.com/gradle-intellij-plugin/#features-392org.jetbrains.grammarkit
bumped to 2022.3: https://github.com/JetBrains/gradle-grammar-kit-plugin/releases/tag/v2022.3com.intellij.flex
+idea_v22.properties
, fixed jvmArgs for runners and numerous adjustments for.gradle
scripts bundledAPI updates
Overall, this refactoring contains runtime error fixes, logger library transition and transitions from not only deprecated, but turned down api. Be aware, these types of changes may break building on older versions entirely
intellij-haxe:main
is completely transitioned toHaxeDebugLogger
to fix clashing with IDEA logger initialization after moving toreload4j
, moduleintellij-haxe:common
left unchanged as constraint violation behaviour was not spottedHaxeLineMarkerProvider
transitioned to updatedLineMarkerInfo
generation, but left withDaemonBundle.message()
unchanged as modifying messages requires standardization.gradle
files include fixes made when booting up main branch from older Gradle, but also includes configuration updates for newer JRE versionREADME.md
support info if neededThis pull request close #1083, close #1096