Skip to content

[#412] use C/C++ language definition from TM4E language pack #413

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

Merged
merged 3 commits into from
Feb 3, 2025

Conversation

ghentschke
Copy link
Contributor

this should fix the lagging multi-line selection via shift+cursor key as well.

fixes #412

this should fix the lagging multi-line selection via shift+cursor key as
well.

fixes eclipse-cdt#412
Copy link
Member

@jonahgraham jonahgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of not having our own copy. Do we need some dependency on tm4e? Perhaps in the feature.xml so that if a user installs CDT LSP they get tm4e?

I think something like this, but I haven't tested it.

$ git diff
diff --git a/features/org.eclipse.cdt.lsp.feature/feature.xml b/features/org.eclipse.cdt.lsp.feature/feature.xml
index e64f8c7..574b0c3 100644
--- a/features/org.eclipse.cdt.lsp.feature/feature.xml
+++ b/features/org.eclipse.cdt.lsp.feature/feature.xml
@@ -29,6 +29,11 @@
       %license
    </license>
 
+   <requires>
+      <import feature="org.eclipse.tm4e.feature"/>
+      <import feature="org.eclipse.tm4e.language_pack.feature" />
+   </requires>
+
    <plugin
          id="org.eclipse.cdt.lsp"
          version="0.0.0"/>

@ghentschke
Copy link
Contributor Author

I like the idea of not having our own copy. Do we need some dependency on tm4e? Perhaps in the feature.xml so that if a user installs CDT LSP they get tm4e?

I agree. I thought that TM4E comes already along with CDT ?

@ghentschke
Copy link
Contributor Author

BTW: I think we should add a dependency to lsp4e as well

@jonahgraham
Copy link
Member

Short answer is dependency management in Eclipse ecosystem is "its complicated":

  • CDT does not come with TM4E
  • Eclipse IDE for C/C++ Developers comes with TM4E (And lots of other stuff)
  • We need feature dependencies (like my proposal) when there is no "hard" MANIFEST requirement and/or when we want to show the features dependency tree in the p2 wizards and UI
  • CDT LSP depends on org.eclipse.tm4e.ui, org.eclipse.tm4e.languageconfiguration bundles, that would pull in (some/all?) of org.eclipse.tm4e.feature, but not org.eclipse.tm4e.language_pack.feature
  • We don't need feature dependency for LSP4E because LSP4E doesn't have a feature and we have the hard dependencies to LSP4E's bundles (and it in turn has dependency on LSP4J)
  • We don't depend on lsp4e.debug, but I don't think anyone is expecting it for C/C++ development
  • The way to test if your dependency is working is starting from eclipse-platform*.zip (from https://download.eclipse.org/eclipse/downloads/) add the CDT LSP update site you want to test + https://download.eclipse.org/releases/2025-03 and then install only cdt-lsp feature and ensure that what you want to have included works

Note that because dependency management in Eclipse ecosystem is complicated, there are lots of errors in it, so this isn't required to change. The Eclipse IDE for C/C++ Developers defines what features are included here so users of the EPP package won't be affected.

@ghentschke
Copy link
Contributor Author

Hm, I think the build job hangs

@ghentschke ghentschke merged commit 98a6aac into eclipse-cdt:main Feb 3, 2025
3 checks passed
@ghentschke ghentschke deleted the remove-doubled-lang branch February 3, 2025 17:33
@jonahgraham jonahgraham added this to the 3.0.0 milestone Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lagging multi-line selection
2 participants