search

ArtsiomCh / CMake

IntelliJ Platform plugin for CMake basic syntax highlight.
https://plugins.jetbrains.com/plugin/10089-cmake-simple-highlighter
GNU General Public License v3.0
44 stars 7 forks source link

CMake simple highlighter plugin prevents CLion 2020.3 from starting #21

Closed fastasturtle closed 3 years ago

fastasturtle commented 3 years ago

Hi @ArtsiomCh,

Dmitry from the CLion team here.

We made some changes for 2020.3, which causes issues when the CMake simple highlighter plugin is installed.

Notably, the CMake support is moved to the plugin, and can no longer be found by the plugin classloader as-is, and the machinery in com.cmakeplugin.utils.CMakePDC doesn't work, causing CMakeFileTypeFactory register a duplicate file type for CLion, etc.

I believe the proper way to address it is to replace

  <depends>com.intellij.modules.clion</depends>

with 

  <depends>com.intellij.clion</depends>

in the plugin.xml which is used for the CLion-compatible version of your plugin (com.intellij.clion is a plugin id, and it allows the classloader to locate classes in this plugin).

Unfortunately, I wasn't able to verify it locally, as I have some issues building the plugin. I'll try harder to investigate if this suggestion doesn't work.

Thank you for the plugin, and sorry for not letting you know sooner.

fastasturtle commented 3 years ago

I've just realized that you probably build the same plugin for all IDEs, and only use <depends> on CLion for local development, is this true?

If so, things have to be changed a bit more (using an optional dependency on CLion), let me experiment and maybe send a draft PR tomorrow (if you won't have a fix by that time).

ArtsiomCh commented 3 years ago

<depends> is commented in plugin.xml and is not used, as far as I remember :) But for Clion (and any other IDE supporting CMake) I heavily depends on com.jetbrains.cmake.* classes. Did you guys move them to another package? PS If you able to build the plugin - let me know how long it takes you to figure out how to do that, please... it's not intendent to be build easily ;)

ArtsiomCh commented 3 years ago

I think I've got the problem. I checked for existence of "com.jetbrains.cmake.CMakeLanguage" class to determine if built-in cmake support presented in this particular IDE. After you've moved your cmake classes into the plugin then your plugin seems to be loaded after my plugin. So my plugin think there's no cmake support and tried to install my CMakeLanguage instance and so on...

Is there any way to check if your cmake plugin presented and make you plugin loads before mine?

ArtsiomCh commented 3 years ago

Fixed at 203.0.2

@fastasturtle will the plugin id "com.intellij.clion" be the same for all other Ide with cmake support? i.e. AndroidStiduio, etc...

fastasturtle commented 3 years ago

Is there any way to check if your cmake plugin presented and make you plugin loads before mine?

I think the most robust way would be to:

It might be convenient to actually move CLion-specific code there as well (so you won't have unresolved references while developing), but that's another story.

fastasturtle commented 3 years ago

will the plugin id "com.intellij.clion" be the same for all other Ide with cmake support? i.e. AndroidStiduio, etc...

AFAIK, for now, CLion is the only IDE where our CMake support is included. It'll be probably moved to a separate plugin like com.intellij.cmake or something during 211 release cycle, we'll let you know (hopefully, in advance).

I believe, for some reason, Android Studio is using the same CMakeListsFileType as CLion does, but doesn't reuse our PSI implementation (please let me know if you observe something different). It doesn't have this plugin id, and it's up to AS team if they want to move CMakeListsFileType to some plugin or not (for now, I believe, no changes are made on their side).

fastasturtle commented 3 years ago

PS If you able to build the plugin - let me know how long it takes you to figure out how to do that, please... it's not intendent to be build easily ;)

I've built it with two half an hour attempts, haven't discovered the dev branch initially :) But the fix for this issue is not yet pushed, right?

If you ever want to make it easier, using gradle-grammarkit-plugin and getting all the IDEs from IJ artifacts repo instead of the local ones might help.

ArtsiomCh commented 3 years ago

I think the most robust way would be to:

Nice solution. I did it a bit simpler way: <depends optional="true" config-file="">com.intellij.clion</depends> and then check in runtime PluginManagerCore.getPlugin(PluginId.getId("com.intellij.clion")) != null

The problem with both your and my solution as it's rely on Clion existence. And AFAIK Cmake support (almost the same as in Clion) presence at Android Studio (with exactly the same PSI at least) and probably for some other IDE (GoLand?). That's why I avoided to check JB cmake support through <depends> previously and instead did check for Class.forName("com.jetbrains.cmake.CMakeLanguage")

I've built it with two half an hour attempts, haven't discovered the dev branch initially :) But the fix for this issue is not yet pushed, right?

;) that's made for purpose. As you can see both free Simple highlighter and paid Plus plugins located in the same open repo (historically and due to GPL). So, I think folks with enough qualification to build it within 1 hour have hourly rate much higher then the Plus plugin annual license :) In the same time I'm willing to "donate" Plus plugin for those who is underpaid, have enough free time and willing to learn how Idea's plugins are made, how java instrumentation works, how serialization works, etc. ;)

fastasturtle commented 3 years ago

And AFAIK Cmake support (almost the same as in Clion) presence at Android Studio

Yes, that's correct, not sure why my initial experiments did show otherwise.

So, for AS, unfortunately, you'll have to support the old approach as well. We'll work with AS team for 211 to have it unified (will let you know when/if this happens).

and probably for some other IDE (GoLand?)

No, only CLion is using this among JetBrains IDEs for now. If this ever changes, again, it would be a unified solution.

;) that's made for purpose.

I see, cool! :) Indeed, my example is not representative because, while I don't really have much experience with building out-of-tree IJ plugins, I probably have above-average knowledge of what's going on :)

ArtsiomCh commented 2 years ago

Update. Due to reported issue with Android Studio 203 I found that trick with <depends optional="true" config-file="">com.intellij.clion</depends> and then check in runtime PluginManagerCore.getPlugin(PluginId.getId("com.intellij.clion")) != null doesn't work in AS >=203 :( Found workaround by checking in runtime PluginManagerCore.getPlugin(PluginId.getId("com.intellij.cidr.base")) != null which make me guess that AS team stay under old com.intellij.cidr namespace. @fastasturtle any chance to ask them unify Cmake related internals with Clion?

maciejmatuszak commented 1 year ago

For anyone trying to use this solution with recent CLion version you may need to skip the config-file="" like this:

<depends optional="true" >com.intellij.clion</depends>

otherwise latest CLion complains:

2023-04-13 23:30:42,599 [     70]   WARN - #c.i.i.p.PluginManager - Cannot load ~/git/ros-integrate/build/idea-sandbox/plugins/ros-integrate/lib/instrumented-ros-integrate-0.1.7.jar
java.lang.StringIndexOutOfBoundsException: String index out of range: 0
    at java.base/java.lang.StringLatin1.charAt(StringLatin1.java:48)
    at java.base/java.lang.String.charAt(String.java:1513)
    at com.intellij.ide.plugins.PluginXmlPathResolver$Companion.toLoadPath$intellij_platform_core_impl(PluginXmlPathResolver.kt:51)
    at com.intellij.ide.plugins.PluginXmlPathResolver.resolvePath(PluginXmlPathResolver.kt:114)
    at com.intellij.ide.plugins.IdeaPluginDescriptorImpl.processOldDependencies(IdeaPluginDescriptorImpl.kt:279)
    at com.intellij.ide.plugins.IdeaPluginDescriptorImpl.readExternal(IdeaPluginDescriptorImpl.kt:249)
    at com.intellij.ide.plugins.PluginDescriptorLoader.loadDescriptorFromJar(PluginDescriptorLoader.kt:152)
    at com.intellij.ide.plugins.PluginDescriptorLoader.loadFromPluginDir(PluginDescriptorLoader.kt:236)
    at com.intellij.ide.plugins.PluginDescriptorLoader.loadDescriptorFromFileOrDir(PluginDescriptorLoader.kt:194)
    at com.intellij.ide.plugins.PluginDescriptorLoader.loadDescriptorFromFileOrDir$default(PluginDescriptorLoader.kt:181)
    at com.intellij.ide.plugins.PluginDescriptorLoader$loadDescriptorsFromDir$1$1$1.invokeSuspend(PluginDescriptorLoader.kt:837)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
    at kotlinx.coroutines.internal.LimitedDispatcher.run(LimitedDispatcher.kt:42)
    at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:95)
    at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:570)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:677)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:664)
2023-04-13 23:30:42,668 [    139]   WARN - #c.i.i.p.PluginManager - Problems found loading plugins:
  File '~/git/ros-integrate/build/idea-sandbox/plugins/ros-integrate/lib/instrumented-ros-integrate-0.1.7.jar' contains invalid plugin descriptor

source: https://youtrack.jetbrains.com/issue/IDEA-270963/Plugin-with-optional-dependency-config-file-in-plugin.xml-not-working-with-IDEA-2021.1.2#focus=Comments-27-5090764.0-0

ArtsiomCh commented 1 year ago

Hi @maciejmatuszak I see (latest only?) Rider has all cmake psi classes (com.jetbrains.cmake.psi) under the hood, but not using them for display source: org.jetbrains.plugins.textmate.psi.TextMateFile is using as PsiFile. Is that a feature or a bug?