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

Lsp upgrade #73

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Lsp upgrade #73

wants to merge 4 commits into from

Conversation

mmitas777
Copy link

This PR adds support for the Language Server Protocol (LSP) to all DSLs. This is done by adding launchers to create and launch language servers basing on the language definitions of the DSLs and the Xtext implementation of a language server. The language servers are available inside runnable Fat JARs that are produced during the LEMMA build.

For demonstrating purposes, extensions for VSCode were built to use the language servers for supporting language features (auto completion, Goto Definition, etc.) outside Eclipse in every LSP-capable IDE. These extensions also include TextMate grammars and a color theme similar to the Eclipse Dark Theme for seamless switch between Eclipse and VSCode.

Marcel Mitas and others added 3 commits October 31, 2023 16:30
… languages (Creation of the corresponding language servers during the LEMMA build process and creation of launchers)
@mmitas777 mmitas777 self-assigned this Nov 3, 2023
@frademacher
Copy link
Collaborator

Hi. First of all, thanks a lot for the PR and the valuable inclusion of the LSP feature and VSCode extensions. Highly appreciated work! And here go my comments with the PR:

commit 46fe281:

  • There are a lot of changes that don't seem to concern the LSP feature, which made it hard to isolate the changes that are relevant to the feature. In general, commits should only comprise those changes that really belong to the feature/bug fix/change introduced by the commit. For instance, the changes to everything in examples/charging-station-management seem to stem from the projects in this location having gotten imported into an Eclipse workspace (hence the .classpath, .project, and .settings filesystem elements). Of course one can do this locally if it helps, but it should not land in a PR that has nothing to do with making those project "Eclipse-ready" but that in fact focuses on enabling LSP for DSLs. Another example are Java files which are generated from Xtend files. Yes, for historical reasons, the repository unfortunately contains Xtend-generated Java files and it would be nice to get rid of them (see https://github.com/SeelabFhdo/lemma/tree/generated_code_removal2 for an PoC on this issue). For now, however, please always commit only those generated Java files that result from changes to Xtend files that are really relevant to the commit, e.g., de.fhdo.lemma.data.datadsl/xtend-gen/de/fhdo/lemma/data/DataDslStandaloneSetup.java resulting from your changes to de.fhdo.lemma.data.datadsl/src/de/fhdo/lemma/data/DataDslStandaloneSetup.xtend but not (for example) de.fhdo.lemma.data.datadsl.metamodel/src-gen/de/fhdo/lemma/data/util/DataSwitch.java, de.fhdo.lemma.data.avro/xtend-gen/de/fhdo/lemma/data/avro/ui/to_avro/LemmaToAvroDialog.java, de.fhdo.lemma.data.intermediate.metamodel/src-gen/de/fhdo/lemma/data/intermediate/IntermediateDataModel.java, or de.fhdo.lemma.utils/xtend-gen/de/fhdo/lemma/utils/RotatingWindowList.java. There are two ways to correct this situation and I'm fine with whatever works best for you: Redo the PR with only those changes that are relevant to the respective commits or list the relevant files for me to review as a response to this one.

  • There are several ServerLauncher.java files (e.g., de.fhdo.lemma.data.datadsl.ide/src/de/fhdo/lemma/data/ide/ServerLauncher.java) and assume them to be relevant to the commit. For consistency, it would however be nice if you could migrate them to Xtend since this is the language in which we implement our DSLs.

commit e7f999e:

commit 724a39d:

  • Since the vscode_extensions folder is part of the LEMMA repository, we don't need separate LICENSE files in it or in any of its sub-folders.

  • Are the .bin folders really necessary for the extensions to work or are they generated by a build process? Some question for all the node_modules folders but here I additionally see the issue that their inclusion might lead to problems with our license.

  • Are the extensions already part of LEMMA's build process? If not, it would be great to add them. First question regarding this task: How are the extensions build, which technologies and CLI commands are used for that? Moving forward with including the extensions, we can have a separate discussion on the build process when all above questions got clarified.

…on of the ServiceDSL and adjust color themes more to Eclipse dark theme
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

2 participants