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

Switch to use Language Configurations and gramars from TM4E Language … #1252

Conversation

vrubezhny
Copy link
Contributor

…Pack

@vrubezhny vrubezhny marked this pull request as draft June 19, 2023 20:03
@vrubezhny
Copy link
Contributor Author

vrubezhny commented Jun 19, 2023

XML Language Configs and Grammars aren't supported in current TM4E Language Pack (0.7.1)

JavaScriptReact (JSX) Syntax Highlighting works, however TestSyntaxHighlighting.testJSXHighlighting fails for some reason.

@HannesWell
Copy link
Contributor

HannesWell commented Aug 19, 2023

Would it be possible to switch to the latest tm4e release even before this greater rework, so that downstream consumers like m2e could use it with the latest snakeyaml?

@vrubezhny
Copy link
Contributor Author

@HannesWell I haven't tested yet, but without switching to use tm4e language pack provided schemas there is no need to drag (require) org.eclipse.tm4e.language_pack.feature to WWD. And just having the latest tm4e release referenced in target platform... do you think it'll help m2e-core (which depends on WWD) to get a dependency to the language pack? Why not to make m2e to depend directly on tm4e/tm4e,language_pack then?

@vrubezhny
Copy link
Contributor Author

It's just that switching to tm4e language_pack would be welcomed, but currently not all the editors can work well with its schemes as well as not all the required scheames are included to tm4e.language_pack, so we cannot switch fully to use tm4e LP at the moment.

@HannesWell
Copy link
Contributor

do you think it'll help m2e-core (which depends on WWD) to get a dependency to the language pack? Why not to make m2e to depend directly on tm4e/tm4e,language_pack then?

It is not urgent. Mainly I was going through m2e's target-platform and noticed that it is not using the latest tm4e dependencies and therefore snakeyaml 1 (which is from Orbit and not Maven-Central, which is why this often needs extra care).
Since I'm not familiar with any of the editor topics I cannot tell if wildwebdevelopers works well with the more recent version of tm4e out of the box.

Btw. I was told for m2e that it is not recommended for projects that participate in the SimRel to use the SimRel repository in their TP because one is basically forming a cycle. I just noticed it is used here and wanted to mention that.

@vrubezhny
Copy link
Contributor Author

It is not urgent. Mainly I was going through m2e's target-platform and noticed that it is not using the latest tm4e dependencies and therefore snakeyaml 1 (which is from Orbit and not Maven-Central, which is why this often needs extra care).

Currently, when WWD target platform is built it takes tm4e from https://download.eclipse.org/releases/latest/ - but there is no any tm4e language pack provided. So in this PR I've switched to directly use tm4e repository (https://download.eclipse.org/tm4e/releases/latest/).
If you believe it's needed I can split this change into a separate PR so, when m2e-core is built it should drag the latest tm4e release for sure, but I'm not sure in it will bring the LP without referencing it in some feature.

Btw. I was told for m2e that it is not recommended for projects that participate in the SimRel to use the SimRel repository in their TP because one is basically forming a cycle. I just noticed it is used here and wanted to mention that.

m2e-core depends on:

  • Lemminx-Maven extension
  • WWD (wwd.xml) (strict version range because of Lemminx and Lemminx-Maven are to be of API-compatible versions )

WWD demends on:

  • Lemminx

I see no problem nor a cycle dependency possibility here. Do you see any problem?

@HannesWell
Copy link
Contributor

If you believe it's needed I can split this change into a separate PR so, when m2e-core is built it should drag the latest tm4e release for sure, but I'm not sure in it will bring the LP without referencing it in some feature.

It was more a suggestion because I assume it would be simpler to accomplish than this entire rework and therefore the latest tm4e can be used earlier.
You don't necessarily have to include it in a feature to drag it into the repo. You can also add it via your category.xml, or even simpler let tycho create a self contained repo, but filter out all bundles that are already contained in other p2-repos and instead add references to that repos instead. I drafted that for m2e already, but this does not yet work perfect and some adjustments in Tycho need to be made: eclipse-m2e/m2e-core#1222

I see no problem nor a cycle dependency possibility here. Do you see any problem?

This was more meant in general if a project that contributes to https://download.eclipse.org/releases/ also uses https://download.eclipse.org/releases/ in its TP, the SimRel repo depends on itself respectively its older versions and it could happen that it contains artifacts that are actually not build anymore and were just bootstrapped via previous releases.
Plus it has the disadvantage that wildwebdeveloper always has to wait for the next Eclipse SimRel to updates its dependencies. If for example tm4e is referenced directly the latest relaesed version can be consumed immediately. So you have more recent artifacts in the wildwebdeveloper repo (which also contains tm4e and from which m2e gets it) and also are more likely to test with the artifacts that tm4e will also contribute to the SimRel. At the moment you are always testing/developing against tm4e from the previous release but in the SimRel wildwebdeveloper will be delivered with the current release.

@vrubezhny
Copy link
Contributor Author

I've split the tm4e dependency to be downloaded from the tm4e repository (instead of Eclipse Releases) out of this PR. This makes WWD to use the latest released tm4e version and gives access to tm4e Language Pack bundle: #1317

@HannesWell
Copy link
Contributor

Great! Thank you. :)

@akurtakov
Copy link
Contributor

Closing this one as some conversion happened already via decec1b ff48fba 796ae27 .
More to come later.

@akurtakov akurtakov closed this May 9, 2024
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.

None yet

3 participants