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] Don't run the generator for open files #2598

Open
spoenemann opened this issue Jun 30, 2017 · 12 comments
Open

[lsp] Don't run the generator for open files #2598

spoenemann opened this issue Jun 30, 2017 · 12 comments

Comments

@spoenemann
Copy link
Member

Currently the generator is triggered for every build, even when a file is opened. The generator should be suppressed for builds triggered by didOpen, any also for files that are open and dirty (a didChange has been received).

@svenefftinge
Copy link
Member

I reconsidered this and now think we should disable the generator always, because we would need two distinct builds to keep file system state separated from in-memory state.
As the LSP is about editing, we should not put both things into one process by default, but rather recommend to run a regular language server without code generation, and have another process that watches the file system for doing code generation.

So for LSP we should bind a NEVER impl of IShouldGenerate.

@cdietrich
Copy link
Member

i prefer at least on option to enable it

@svenefftinge
Copy link
Member

That would mean we trigger file changes because of in-memory document changes, which is not a good idea.

@cdietrich
Copy link
Member

maybe moving to org.eclipse.xtext.ide.server.LanguageServerImpl.didSave(DidSaveTextDocumentParams) ?

@svenefftinge
Copy link
Member

No, the problem is, that we only have one representation per URI and if it is opened, we will use the in-memory state, which would always be wrong for code generation.
The only way to solve this is to have two representations (file and in-memory) which boils down to two builds and two resourcesets. In that case I don't see an advantage of doing it as part of the language server.

@cdietrich
Copy link
Member

hmmm but if we save the represenations are equals?
otherwise you would always have to introduce a custom command to generate?

@svenefftinge
Copy link
Member

svenefftinge commented Jul 5, 2017

One could save one file but keep another one opened, which might be affected.

@svenefftinge
Copy link
Member

We could say we generate only if there are no open files with changes, but that is counter intuitive for the user.

@svenefftinge
Copy link
Member

svenefftinge commented Jul 5, 2017

re: introduce a command
I would not do it with a command, but by running it on the command line, npm or gradle.

@cdietrich
Copy link
Member

hmmm am not convinced if we want to replace e.g. eclipse integration with lsp4e one day for example

@cdietrich
Copy link
Member

cdietrich commented Dec 17, 2018

am not sure if the current IncrementalBuilder is too "stupid" to handle such a case.
if we could tell in build request if a generation should happen.
then we can do the generation in initial build and on save (which we currently ignore in lsp)
and do not generator during document changes
(am not sure what about side usecases like file gets green during change on other file)

unfortunately org.eclipse.xtext.ide.server.ProjectManager.newBuildRequest(List<URI>, List<URI>, List<Delta>, CancelIndicator) to support that
is quite bad api wise. am not sure if we can change it

@svenefftinge
Copy link
Member

As said before, we could only generate if all files are in sync with the file system, because a just saved file could reference a dirty document that affects the code generation.
I imagine that it will be surprising or at least non-intuitive for users if we only generate once all editors are saved.
Depending on the actual language and code generator this might not be an issue. I just would like to have a sensible default behavior.

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

No branches or pull requests

3 participants