Bug 518650 - activate language server for specific editor independent of the content-type
Summary: activate language server for specific editor independent of the content-type
Status: NEW
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: LSP4E (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-22 11:59 EDT by Martin Lippert CLA
Modified: 2022-02-04 09:12 EST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Lippert CLA 2017-06-22 11:59:35 EDT
we have an interesting situation and I would like to start a discussion about what the best solution for this could be.

We have a language server that can deal with specific YAML files (not YAML files in general). In VS Code, we defined a specific language ID for that and it matches against certain file name patterns. If this doesn't work, the user can select the language ID for an open editor himself to activate the right language server.

For the Eclipse side, we have a content-type defined for our specific YAML files. Since Eclipse can't define content-types against file name patterns, we set it to one specific file name and let the user do "open with" our specialized editor, if the file is called differently.

LSP4E seems to use the content-type only (at the moment) to select with language server to activate for a file when it is being opened in an editor. But now we have the problem that our language server is activated whenever any YAML file is being opened by an editor. There is no way for us to get it activated only for specific files or only when the user selects "open with" (so somewhat bound to our editor instead of the content-type).

We could also try to specialize the content-type, so that it doesn't match all those other files, but I have no idea how to make that work in Eclipse. And it would not solve the question how the user could switch the content-type of an opened file in the editor (ours or the generic one), if the user thinks it should be something specific.

Do you have any thoughts on this? Any ideas what the best way would be to realize this?
Comment 1 Martin Lippert CLA 2017-06-23 07:12:36 EDT
I experimented with this a bit and I am able to activate a language server for a specific content-type from my editor, even though the editor input is not identified of being of that content-type. I just manually call the lsp4e API (similar to what the setup participant is doing.

That is great and the language server is shutdown automatically when I close the editor again.

Unfortunately the other pieces of LSP4E don't work, since they don't find the language server that I activated. The lookup is going through the content-type of the actual file again.

I think it would make sense to change this, so that the LSP4E pieces can use already running language servers for a document in addition to looking up additional language servers (in case they don't run yet).
Comment 2 Martin Lippert CLA 2017-06-23 07:13:57 EDT
VSCode allows the user to change the language ID that the editor should use for the open file (which triggers the matching language servers to be used). Maybe we need something similar in LSP4E.
Comment 3 Mickael Istria CLA 2017-06-23 07:54:16 EDT
In your initial report, you mentioned that Eclipse content-types don't allow fine-grained regexp to decide content-type according to the name. I believe adding this would be the perfect fix on the right layer.
Please open another bug against Eclipse Platform to ask for this (and make me a CC as content-types are part of my current targets).

> In VS Code, we defined a specific language ID for that and it matches against certain file name patterns. If this doesn't work, the user can select the language ID for an open editor himself to activate the right language server.
> [...]
> And it would not solve the question how the user could switch the content-type of an opened file in the editor (ours or the generic one)
> [...]
> VSCode allows the user to change the language ID that the editor should use for the open file (which triggers the matching language servers to be used). Maybe we need something similar in LSP4E.

I'd rather make things good enough to not have the users needing to switch the language. Users switching editor behaviour because the editor identified the wrong type or started the wrong tool isn't a feature, it's a workaround for a bogus detection.


In your case, I can suggest -as a workaround- that you put the logic in the language server: let the language server start and be notified of changes, but ignore everything that's not matching the file name regexp.
The issue is obviously that the server is started, but as it wouldn't return anything interesting for plain yaml files, it shouldn't affect too much end-users.
Another drawback is that in some places such as hover, there is currently no "composition" of results from multiple LS. So if 2 LS provide the hover, only one will be queried. There are tickets such as #516161 on this topic. Those are strong limitations of LSP4E that will need to be tackled at some point; and it seems like we reached this point ;)
Comment 4 Martin Lippert CLA 2017-06-24 07:47:38 EDT
(In reply to Mickael Istria from comment #3)
> In your initial report, you mentioned that Eclipse content-types don't allow
> fine-grained regexp to decide content-type according to the name. I believe
> adding this would be the perfect fix on the right layer.
> Please open another bug against Eclipse Platform to ask for this (and make
> me a CC as content-types are part of my current targets).

The content type limitation is captured in this old bug:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=263316
(I added you to the CC list)

> > VSCode allows the user to change the language ID that the editor should use for the open file (which triggers the matching language servers to be used). Maybe we need something similar in LSP4E.
> 
> I'd rather make things good enough to not have the users needing to switch
> the language. Users switching editor behaviour because the editor identified
> the wrong type or started the wrong tool isn't a feature, it's a workaround
> for a bogus detection.

I agree and I would like to make those definitions as good as possible, so that the best behavior is chosen automatically (without the need to switch editors). Absolutely no question about that.

But I also think that it would be extremely annoying for users if the IDE made a wrong decision about this (even though it might be unlikely), that there is no chance for the user to override this and choose something explicitly. I always hate it when the magic of the IDE is wrong for my case and I have no way to workaround that (from the users perspective).

So making it explicit which language servers got activated for a file (when opened in the generic editor, for example) would be a great step in that direction (and a topic for a separate bug). And then allowing users to opt-out from a chosen language server and/or opt-in into a non-preselected language server would be good. Maybe not on the level of language server, maybe more on the level of language IDs, content-types, or something else.

> In your case, I can suggest -as a workaround- that you put the logic in the
> language server: let the language server start and be notified of changes,
> but ignore everything that's not matching the file name regexp.
> The issue is obviously that the server is started, but as it wouldn't return
> anything interesting for plain yaml files, it shouldn't affect too much
> end-users.

We can do that, would certainly be possible, but would move the magic into the language server (instead of the IDE) and would leave even less choice to the user to switch it on/off. Hmmm...

I still think that the mechanics of starting up a language server based on the content-type is a good way of doing it. But I also think that the other parts (those that use the running language servers, like hover support, content-assist, diagnostics, etc.) should not depend on that decision, but should instead use every language server that is running and active for that file.

I think I will take a look at the code to see what changes would be necessary for this, so see if this fits the design (or makes it even better)... :-)

> Another drawback is that in some places such as hover, there is currently no
> "composition" of results from multiple LS. So if 2 LS provide the hover,
> only one will be queried. There are tickets such as #516161 on this topic.
> Those are strong limitations of LSP4E that will need to be tackled at some
> point; and it seems like we reached this point ;)

Agree!!!
Comment 5 Martin Lippert CLA 2017-06-26 08:28:50 EDT
I think I would like to refactor the code so that there is a single place in the code which can associate files with language servers.

At the moment, which would happen via content-types only, in the future we could enhance this with per-file settings, user preferences, language IDs, etc.
Comment 6 Eclipse Genie CLA 2017-06-26 08:56:52 EDT
New Gerrit change created: https://git.eclipse.org/r/100047
Comment 7 Martin Lippert CLA 2017-06-26 09:00:22 EDT
I made a small step forward and submitted a patch that allows already running and the the document connected language servers to be used by content-assist, hovers, etc.

This allows others (like my manifest editor) to start a language server for a specific file (independent of its actual content-type). This might be a rare case, but it starts to decouple the content-type definition of a file of the actual running language servers.

The default mechanism should still be content-type based, but if a document is already connected to a language server, the language server should also be used when that specific connected file is edited. The other mechanics around document sync and file buffers also rely on that association, that is why this change seems to make sense to me.
Comment 9 Martin Lippert CLA 2017-06-26 09:54:43 EDT
Wow, thanks a lot for this super quick turnaround and merge. Much appreciated!!!
Comment 10 Mickael Istria CLA 2017-06-26 09:58:32 EDT
(In reply to Martin Lippert from comment #9)
> Wow, thanks a lot for this super quick turnaround and merge. Much
> appreciated!!!

Well, thanks for the small, safe and backward compatible patch. And also thanks to all contributors (including yourself) who agree to add tests for any feature. It really boosts agility.
Comment 11 Eclipse Genie CLA 2017-06-29 13:37:54 EDT
New Gerrit change created: https://git.eclipse.org/r/100397
Comment 13 Mickael Istria CLA 2017-08-21 09:10:50 EDT
Is there anything else to do on this topic, or is the provided patch enough to cover your needs?
Comment 14 Martin Lippert CLA 2017-08-21 09:26:54 EDT
I think the only remaining piece is to find the right languageID for resources that are connected that way. I think this is a whole in the current implementation - and should get fixed.
Comment 15 Mickael Istria CLA 2021-11-16 15:55:57 EST
Eclipse LSP4E is moving away from this bugs.eclipse.org issue tracker to https://github.com/eclipse/lsp4e/issues/ instead. If this issue is relevant to you, your action is required.
0. Verify this issue is still happening with latest LSP4E snapshots (from https://download.eclipse.org/lsp4e/snapshots )
  if issue has disappeared, please change status of this issue to "CLOSED WORKFORME" with some details about your testing environment and how you did verify the issue; and you're done
  if issue is still present with snapshots:
* Create a new issue at https://github.com/eclipse/lsp4e/issues/
  ** Use as title in GitHub the title of this Bugzilla ticket (may include the bug number or not, at your own convenience)
  ** In the GitHub description, start with a link to this bugzilla ticket
  ** Optionally add new content to the description if it can helps towards resolution
  ** Submit GitHub issue
* Update bugzilla ticket
  ** Add to "See also" property (up right column) the link to the newly created GitHub issue
  ** Add a comment "Migrated to <link-to-newly-created-GitHub-issue>"
  ** Set status as CLOSED MOVED
  ** Submit

All issues that remain open will be automatically closed soon. Then the Bugzilla component for LSP4E will be archived and made read-only.