Add default language descriptions set

[dkuleshov]

Signed-off-by: Dmytro Kulieshov dkuliesh@redhat.com

What does this PR do?

After a deep and long analysis the decision is to add standard set of language descriptions while more appropriate and more complex solution is yet to come under a separate issue (issues). The way the new languages are added stays the same (via guice configuration of LanguageDescreption) and if no match is found then additional default set of languages is taken in consideration.

Example of a definition of a language server in a workspace configuration (not changed)

"servers":{
   "ls":{
      "attributes":{
         "internal":"true",
         "type":"ls",
         "config":"{\"id\":\"github.com/sourcegraph/go-langserver\", \"languageIds\":[go]}"
      },
      "protocol":"tcp",
      "port":"4417"
   }
}

Where languageIds is a list of language that this server is to be associated with.

What issues does this PR fix or reference?

#7790

Added default set of language descriptions.

Release Notes

Docs PR

[dkuleshov]

ci-build

[codenvy-ci]

Build # 4415 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/4415/ to view the results.

[benoitf]

@dkuleshov the issue was to move config out of source code but you're saying

The way the new languages are added stays the same (via guice configuration of LanguageDescreption)

so it's not possible with configuration to tell which file extension to use and which script to execute to launch the language server ?

[slemeur]

Don't merge before @benoitf concerns are addressed.

[dkuleshov]

@benoitf Thank you for your question.

which script to execute to launch the language server

Language server launch process is out of the scope of this issue.

it's not possible with configuration to tell which file extension to use

The answer would be both yes and no.

It is possible to do that for all languages that are mentioned here https://code.visualstudio.com/docs/languages/identifiers and a few more, our documentation will cover that list. All you need is to set a language that language server corresponds to in a workspace configuration. In the example

"servers":{
   "ls":{
      "attributes":{
         "internal":"true",
         "type":"ls",
         "config":"{\"id\":\"github.com/sourcegraph/go-langserver\", \"languageIds\":[go]}"
      },
      "protocol":"tcp",
      "port":"4417"
   }
}

you're saying that this language server is a go server, so any file that corresponds to go language will be associated with this server.

However that does not work with languages that are not covered by default language list, for such use cases the workflow stays the same as it is now.

To my regret, general solution that will allow us to register language server for arbitrary files in a configuration looks quite challenging at the moment and probably requires a few more tasks to be completed before. In order to be able to register an arbitrary language server we either need to define a corresponding regular expression to directly find files related to the language server or somehow to define the correspondence between the language server and the language to indirectly find matches between files and language servers. The cornerstone in this matter is LanguageDescription. The first option will require significant redesign or removal of LanguageDescription as a concept which, in turn, will entail a lot of changes to language server related stuff because it's a key entity not only for a server but for a client side as well. While the second option requires thorough thinking on a language registry concept and probably general registry concept, which itself is quite a complex topic, because we'll have to provide a simple way to extend the language/language server list.

Therefore, the idea of this pull request is to provide the solution, the preliminary version that will give us quite enough functionality until we implement more appropriate one and it is not expected to solve the issue completely.

[dkuleshov]

ci-build

[vparfonov]

@slemeur could you please tell me why are you blocking the merge, I see that @benoitf is not a code owner here, he's just asking questions

[benoitf]

thanks @dkuleshov for the answer

my concern is that IMHO, all configuration could be done through configuration.

when I look at almost all LS server plugins

1. First, they define the LanguageDescription
   something with

  LanguageDescription description = new LanguageDescription();
        description.setFileExtensions(asList(EXTENSIONS));
        description.setLanguageId(LANGUAGE_ID);
        description.setMimeType(MIME_TYPE);
        Multibinder.newSetBinder(binder(), LanguageDescription.class)
            .addBinding()
            .toInstance(description);

so it's easy to map that to configuration properties as we've string objects

but then I don't see the "MimeType" or the "FileExtensions" properties based on your example

2. next we've to define a LanguageServerLauncherTemplate

and in most cases, we define:

• path of the binary to execute
• we provide a LanguageServerDescription object which is quite close to LanguageDescription

but we could still also map this configuration with json and also merge some content from LanguageDescription so by having in json configuration

{
  "language-server": [
    {
      "path": "$HOME/my-lsp/launch.sh"
    },
    {
      "description": [
        {
          "language-id": "example"
        },
        {
          "regexp": ".*\\.foo"
        },
        {
          "file-extensions": "foo"
        },
        {
          "mime-type": "text/x-foo"
        }
      ]
    }
  ]
}

or with yaml:

language-server:
  - path: "$HOME/my-lsp/launch.sh"
  - description:
    - language-id: "example"
    - regexp: ".*\\.foo"
    - file-extensions: "foo"
    - mime-type: "text/x-foo"

we could bind the LanguageDescription and create default LanguageServerLauncherTemplate with its custom LanguageServerDescription

[benoitf]

also by turning my LSP into a container we don't need anymore the path as it is handled by the container startup.

Then I could simply apply Docker labels into the docker image like

LABEL CHE_LANGUAGE_SERVER_LANGUAGE_ID="example"
LABEL CHE_LANGUAGE_SERVER_REGEXP=".*\\.foo""
LABEL CHE_LANGUAGE_SERVER_FILE_EXTENSIONS="foo"
LABEL CHE_LANGUAGE_SERVER_MIME_TYPE="text/x-foo"

and just reference my LSP docker image

[dkuleshov]

@benoitf I see your point, that is one of possible options that we're thinking about, however there are yet several important things that must be addressed first.

If you can see, path matchers are defined in a DocumentFilter class and have each own language they corresponds to. DocumentFilter class itself is used fo multi language server use cases, so we have to develop the way we will distinguish language id in the language description and language id in the document filter (which is a part of language description itself). I mean the mapping could be not as easy as it seems when we take into account existing language server data model, on the other hand language server related components data model redesign may have other consequences.

Another possible issue is the situation when we have several language servers that define languages that are similar in some way or the same, which language definition should be taken or how we can merge it. We can't just have several language descriptions pointing to one language but we also can't have one language description for several different language/dialects. I mean it seems for me not a good idea to mix language server definition and language definition.

I'm still have no idea why are we discussing language server launching workflow here.

[benoitf]

@dkuleshov in Eclipse Che, how many LSP servers are not following a simple approach for registering themselves?
I mean, do we we have many LS server that are not only providing mime type, language id, file extension, regexp and path of LS to launch (with LSP container can be omitted) ?

[benoitf]

Note also that for a JUG, I wrote my Mickael Istria my own LSP server implementation and the files that we are associating are "*.ski"

Fom what I see, it's not recognized in DefaultLanguages so it will probably fail if I try to use only the json configuration.
While the LS server implementation is really dummy

[dkuleshov]

@benoitf I don't think we have any.

I would say that the only mandatory parameter should be regexp, while the others may be removed at all (not speaking about launching, here we will probably need ls path or ls image name or something, not the case here) or left as metadata. I don't see why we should keep LenguageDescription as it is used only to find a match for a language server. Regular expression should be just enough to define which LS should correspond to which file. I'm not 100% sure whether for client side it is enough to have regexp (without mime type) but seems it is.

The configuration would be similar to this:

"servers":{
   "typescript-ls":{
      "attributes":{
         "internal":"true",
         "type":"ls",
         "regexp":".*\\.ts"
      },
      "protocol":"tcp",
      "port":"4417"
   }
}

That would simplify language server configuration and language server data model greatly, however this approach probably will require significant rework of current language server related workflows. That is my biggest concern here, that's why I would suggest the solution from this pull request for the initial stage.

[codenvy-ci]

Build # 4418 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/4418/ to view the results.

[benoitf]

I prefer this updated solution with regexp pattern.
BTW I don't think there is a need of a big "significant rework of current language server related workflows" if you're able to register programmatically LanguageDescription, LanguageServerDescription from a LauncherTemplate automatically.

[dkuleshov]

@benoitf I'm not sure that I'm following your words, perhaps it would be clearer for me if you could show me a code snippet of a solution that you suggest.

[benoitf]

@dkuleshov
ok, so

1. from the JSON you are able to build LanguageDescription objects and register objects into LanguageServerRegistryImpl
2. from he JSON you're also able to build LanguageServerDescription/LanguageServerLauncherTemplate(implementation of LanguageServerLauncher) objects and register them into LanguageServerRegistryImpl as well

so when client will ask server, it will find the new objects registered by workspace JSON.

[dkuleshov]

@benoitf I apologize, apparently I'm not smart enough because I'm still don't see what specifically you're trying to say. I mean when you're saying

from the JSON you are able to build LanguageDescription objects and register objects into LanguageServerRegistryImpl

of course we can build LanguageDescription object from a JSON because it is a DTO but you didn't say anything specific to our discussion, so that does not seem productive to me.

I still don't see an easy way how we can make all necessary things done properly and not making another workaround. But if you have a clear vision and I believe you have it, and if you're saying that it's not a big rework, probably making a pull request with your solution would be not a big work as well. There we could discuss your decision on the merits.

[benoitf]

@dkuleshov ok more simple

We're able to create objects from JSON and register correctly these objects (including LanguageDescription instances, etc)

so, why would we require users to write these instances if by adding some config, Eclipse Che can generate it automatically for users ?
And if you're in a 5% of case where you really need a plugin, you could still write the plugin without adding the json configuration

[dkuleshov]

@benoitf I'm not asking for a more simple explanation, I'm asking for a more specific explanation. If you're saying that we don't need a big significant rework as an alternative solution, I humbly assume that the solution is an insignificant rework. Hence it should not be a big deal for you to define the solution in a more specific (not abstract) way as is customary in our community, for example as a pull request.

[benoitf]

@dkuleshov
you asked for a specific explanation.

This PR only provides a default list of file extensions with mimetype
so for me the title "Move language description configuration out of source code" is not correct as it's again all in the source code. It just tried to be a big list but it is not complete and can't handle all usecases.
Foe example, if I require a language server for a file extension that is not defined in this "hardcoded list" then it fails as I cannot register simply a language description configuration without source code

[dkuleshov]

Changed the title 👍

[dkuleshov]

ci-build

[codenvy-ci]

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/4419/

[vparfonov]

ci-test

[vparfonov]

@slemeur could you please tell me why are you blocking the merge, I see that @benoitf is not a code owner here, he's just asking questions

[slemeur]

@vparfonov : I had the same concerns than @benoitf and just didn't want the work being merged without seeing a discussion on it. It's fine for me now. Thanks @dkuleshov for handling it.

BTW in the latest comment:

For example, if I require a language server for a file extension that is not defined in this "hardcoded list" then it fails as I cannot register simply a language description configuration without source code

@dkuleshov : Are you going to take care of this use case? I should be considered as a pretty nominal case. If so, could you please create the corresponding issue and link it to this PR ?

Thanks.

[codenvy-ci]

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:8256
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

[musienko-maxim]

Selenium cycle did not find new regressions

[dkuleshov]

@slemeur The issue is not resolved yet, after this pull request is merged with master the work on the issue will be proceeded.