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

Add LSP supporting libraries #1476

Closed
wants to merge 1 commit into from

Conversation

laeubi
Copy link
Contributor

@laeubi laeubi commented Oct 26, 2023

Will be required by

@mickaelistria @akurtakov would the be general concerns to add LS4e/j to the platform target? I think the sooner or later they end up in the EPPs anyways because of m2e or others using LSP...

@akurtakov
Copy link
Member

My only concern is yet another circular dependency (platform<-> lsp4e) in the same build.
On the other side I see it inevitable that PDE uses LSP stack .
If only PDE was truly separate that wouldn't be an issue as PDE sits on top of everything.

@laeubi
Copy link
Contributor Author

laeubi commented Oct 26, 2023

I think lsp4j is only part of simrel but not of aggregator? And of course platform should only reference latest release of lsp4x API ...

@merks
Copy link
Contributor

merks commented Oct 26, 2023

Note that ``http://download.eclipse.org/lsp4e/releases/latest` is a moving target while the versions are locked in exactly. That looks like a good formula for having the build suddenly break.

Also note that the target platform is in slicer mode so what other dependencies are dragged in?

SimRel currently has this:

image

It's not https://download.eclipse.org/lsp4e/releases/0.24.3/ as one might expect or hope.

The following looks like just the tip of the iceberg in terms of dragging new dependencies that one will need in order to lock in to all specific versions transitively in slicer mode:

image

Do "we" really want to turn the Platform (PDE) into a mini-simrel? How will that impact publishing to Maven central?

And then there is the "circular dependency" question which would suddenly because a long and deep cycle:

#1472

This looks like a slippery slope to me.

@akurtakov
Copy link
Member

Definetely not smth I would like to see in Platform. PDE on the other side is a separate project which could benefit enormously from LSP and in general not having same restrictions as Platform.
Technically, PDE is an integration plugin that is closer to M2E in terms of target audience than rest of current releng.aggregator thus it handling things in the way that it satisfies users most makes more sense than putting Platform style restrictions on it.

@mickaelistria
Copy link
Contributor

+1 with @merks and @akurtakov here, I don't think we want to give any chance for LSP4E to leak into Platform, but for JDT or PDE UI it'd be fine. But it would require to have distinct p2 repos and distinct target files for Platform/JDT/PDE (which would IMO be a good step forward).

@laeubi
Copy link
Contributor Author

laeubi commented Oct 26, 2023

I must confess I don'T understand how it makes a difference to include it in PDE then here as long as PDE is part of the aggregator build but for sure I can add it to PDE without a problem, Tycho can do that without a problem but will Oomph be able to handle such target split?

@laeubi
Copy link
Contributor Author

laeubi commented Nov 5, 2023

See linked issues:

Having Eclipse-Platform not able to include/use LSP4e (e = Eclipse IDE no?) a technique that is made to integrate with IDEs and instead require each project to find its own way to essentially make it part of Eclipse again sound like we would not include EMF, E4, or others, also I think this is a big disadvantage for user and plugin developers compared to VSCode or other that seem to ship this by default as their key technology to integrate languages...

/CC @jonahgraham @kthoms

@akurtakov
Copy link
Member

akurtakov commented Nov 5, 2023

Let's have PDE split up (as started in eclipse-pde/eclipse.pde#826) and then we can have PDE depend on LSP stack. IMHO, this is a good goal for 2024-03 release.

@laeubi
Copy link
Contributor Author

laeubi commented Nov 5, 2023

I also like to add the quote from @mickaelistria here

Every text file format should be handled by a LS.

So also .project, .classpath and many other fall in this category.

@szarnekow
Copy link

I also like to add the quote from @mickaelistria here

Every text file format should be handled by a LS.

Language servers cannot talk to each other without jumping through hoops. Since the I of IDE is for Integrated, I don’t by that bold claim. LS are not a silver bullet.

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.

5 participants