[WIP] Several improvements to the way remote language servers may be configured through workspace configuration #9387

dkuleshov · 2018-04-12T09:12:16Z

Related issue: #8925

The way we configured remote language servers was not really convenient and limited to the list of hard coded supported language identifiers that corresponded to LSP specification. The idea of this pull request is to make some little changes to language server configuration so it would be more flexible and simple. While the configuration took just a little changes the internal architecture due to its limited capacity to be changed was reorganized significantly more. Configuration and most noticeable architecture changes will be covered further.

Workspace language server configuration

Previously the server attributes configuration section looked like:

"attributes":{
   "internal":"true",
   "type":"ls",
   "config":"{\"id\":\"org.eclipse.che.plugin.cpp\", \"languageIds\":[\"c\", \"cpp\"], \"documentFilters\":[  {  \"languageId\":\"cpp\",  \"pathRegex\":\".*\\\\.(c|cpp|h)\"} ]}"
}

Language server identifier moved to server attributes section, seems like it (along with the type) is more an attribute than a configuration of a server. Language IDs and document filters sections unfortunately do not work robustly due to their complexity, are not flexible enough and seems excessive in context of using language IDs, so they are replaced with more plain and powerful (as it seems) language regular expressions section. The new equivalent configuration snippet should look like:

"attributes":{
   "internal":"true",
   "type":"ls",
   "id":"org.eclipse.che.plugin.cpp",
   "languageRegexes":"[ {\"languageId\":\"c\", \"regex\":\".*\\\\.(c|h)$\"},  {\"languageId\":\"cpp\", \"regex\":\".*\\\\.(cpp|hh)$\"}]"
}

So, eventually the newly modified workspace configuration taken out of this pull request should be:

{
  "environments": {
    "default": {
      "machines": {
        "typescript-ls-machine": {
          "attributes": {
            "memoryLimitBytes": "1073741824"
          },
          "servers": {
            "ls": {
              "attributes": {
                "languageRegexes": "[ {\"languageId\":\"typescript\", \"regex\":\".*\\\\.(ts|tsx)$\"}]",
                "internal": "true",
                "id": "ts-id",
                "type": "ls"
              },
              "port": "4417",
              "protocol": "tcp"
            }
          },
          "volumes": {
            "projects": {
              "path": "/projects"
            }
          },
          "installers": [
            "org.eclipse.che.exec",
            "org.eclipse.che.terminal"
          ],
          "env": {}
        },
        "dev-machine": {
          "attributes": {
            "memoryLimitBytes": "2147483648"
          },
          "servers": {
            "tomcat8-debug": {
              "attributes": {
                "some": "attribute"
              },
              "port": "8000",
              "protocol": "http"
            },
            "codeserver": {
              "attributes": {},
              "port": "9876",
              "protocol": "http"
            },
            "tomcat8": {
              "attributes": {},
              "port": "8080",
              "protocol": "http"
            }
          },
          "volumes": {
            "projects": {
              "path": "/projects"
            }
          },
          "installers": [
            "org.eclipse.che.exec",
            "org.eclipse.che.terminal",
            "org.eclipse.che.ws-agent",
            "org.eclipse.che.ssh"
          ],
          "env": {}
        }
      },
      "recipe": {
        "type": "compose",
        "content": "services:\n typescript-ls-machine:\n  image: eivantsov/tsls:0.1.11\n  environment:\n   MYSQL_ROOT_PASSWORD: password\n   MYSQL_DATABASE: petclinic\n   MYSQL_USER: petclinic\n   MYSQL_PASSWORD: password\n  mem_limit: 1073741824\n dev-machine:\n  image: eclipse/ubuntu_jdk8\n  mem_limit: 2147483648\n  depends_on:\n   - typescript-ls-machine\n",
        "contentType": "application/x-yaml"
      }
    }
  },
  "defaultEnv": "default",
  "name": "TypeScript_LS_Demo",
  "projects": [
    {
      "name": "pc",
      "attributes": {
        "contribute_to_branch": [
          "mysql"
        ],
        "commands": [
          "{\"commandLine\":\"mvn -f /projects/pc clean install -DskipTests\", \"name\":\"pc: build\", \"attributes\":{\"previewUrl\":\"\", \"goal\":\"Build\"}, \"type\":\"mvn\"}",
          "{\"commandLine\":\"$TOMCAT_HOME/bin/catalina.sh stop\", \"name\":\"pc: stop tomcat\", \"attributes\":{\"previewUrl\":\"\", \"goal\":\"Run\"}, \"type\":\"custom\"}",
          "{\"commandLine\":\"mvn -f /projects/pc clean install -DskipTests \\ncp /projects/pc/target/*.war $TOMCAT_HOME/webapps/ROOT.war \\n$TOMCAT_HOME/bin/catalina.sh run 2>&1\", \"name\":\"pc: build and deploy\", \"attributes\":{\"previewUrl\":\"${server.tomcat8}\", \"goal\":\"Run\"}, \"type\":\"mvn\"}",
          "{\"commandLine\":\"mvn -f /projects/pc clean install -DskipTests \\ncp /projects/pc/target/*.war $TOMCAT_HOME/webapps/ROOT.war \\n$TOMCAT_HOME/bin/catalina.sh jpda run 2>&1\", \"name\":\"pc: debug\", \"attributes\":{\"previewUrl\":\"${server.tomcat8}\", \"goal\":\"Debug\"}, \"type\":\"mvn\"}"
        ]
      },
      "links": [],
      "type": "maven",
      "source": {
        "location": "https://github.com/che-samples/web-java-spring-petclinic.git",
        "type": "git",
        "parameters": {
          "branch": "mysql"
        }
      },
      "path": "/pc",
      "problems": [],
      "mixins": [
        "pullrequest"
      ]
    }
  ],
  "commands": [
    {
      "commandLine": "mysql -u $MYSQL_USER -p$MYSQL_PASSWORD -e 'show databases;'",
      "name": "show databases",
      "attributes": {},
      "type": "custom"
    }
  ],
  "links": []
}

Guice language server configuration

In order to keep all related to language server configuration in one place new interface org.eclipse.che.api.languageserver.LanguageServerConfig is introduced. It consists of several sections that define how language server is matched upon workspace item's paths, how LangaugeServer instance is created, and how the communication between LanguageServer and local language server process or remote language server happens. Those aspects are correspondingly represented by org.eclipse.che.api.languageserver.LanguageServerConfig.RegexProvider, org.eclipse.che.api.languageserver.LanguageServerConfig.InstanceProvider and org.eclipse.che.api.languageserver.LanguageServerConfig.CommunicationProvider and must be configured to run the language server.

The simplest example of such configuration is org.eclipse.che.plugin.php.languageserver.PhpLanguageServerConfig:

public class PhpLanguageServerConfig implements LanguageServerConfig {

  private static final String REGEX = ".*\\.php";

  private final Path launchScript;

  @Inject
  public PhpLanguageServerConfig() {
    this.launchScript = Paths.get(System.getenv("HOME"), "che/ls-php/launch.sh");
  }

  @Override
  public RegexProvider getRegexpProvider() {
    return new RegexProvider() {
      @Override
      public Map<String, String> getLanguageRegexes() {
        return ImmutableMap.of(PhpModule.LANGUAGE_ID, REGEX);
      }

      @Override
      public Set<String> getFileWatchPatterns() {
        return ImmutableSet.of();
      }
    };
  }

  @Override
  public CommunicationProvider getCommunicationProvider() {
    ProcessBuilder processBuilder = new ProcessBuilder(launchScript.toString());
    processBuilder.redirectInput(ProcessBuilder.Redirect.PIPE);
    processBuilder.redirectOutput(ProcessBuilder.Redirect.PIPE);

    return new ProcessCommunicationProvider(processBuilder, PhpModule.LANGUAGE_ID);
  }

  @Override
  public InstanceProvider getInstanceProvider() {
    return DefaultInstanceProvider.getInstance();
  }
}

Where DefaultInstanceProvider is the default implementation that reflects most common way of LangaugeServer instance creation and is suitable for most cases but can be replaced by a custom implementation where needed. ProcessCommunicationProvider provides appropriate InputStream and OutputStream instances for communication of proxy instance and language server, along with lazy language server process start.

There is a specific binding in Guice for such configurations:

newMapBinder(binder(), String.class, LanguageServerConfig.class)
        .addBinding("org.eclipse.che.plugin.php.languageserver")
        .to(PhpLanguageServerConfig.class);

Where "org.eclipse.che.plugin.php.languageserver" is the identifier of a corresponding language server.

In case of adding language server through workspace configuration the same approach is used: a corresponding instance of LangaugeServerConfig is automatically created, org.eclipse.che.api.languageserver.SocketCommunicationProvider is used instead.

Language descriptions

Since org.eclipse.che.api.languageserver.shared.model.LanguageDescription mostly used as a container for language related mime types, extensions and highlighting configuration which are not directly related to language servers (as of 3.6.0 LSP specification) but to editor configuration, all language descriptions moved to client side, the most basic example:

public class PhpLanguageDescriptionProvider implements Provider<LanguageDescription> {
  private static final String LANGUAGE_ID = "php";
  private static final String[] EXTENSIONS = new String[] {"php"};
  private static final String MIME_TYPE = "text/x-php";

  @Override
  public LanguageDescription get() {
    LanguageDescription description = new LanguageDescription();
    description.setFileExtensions(asList(EXTENSIONS));
    description.setLanguageId(LANGUAGE_ID);
    description.setMimeType(MIME_TYPE);

    return description;
  }
}

Language server matching

It unfortunately appeared that the implementation of LanguageServerRegistry in its current state is not as effective as expected and has some inconvenient limitations for language server matching routines, that is why in this pull request it is partially removed and will probably require improvements.

First of all, despite an excellent idea of using numbers to accurately find matching language server score, it can have only one of three states - 0, 5, 10 - and all of them are treated as matches and impacts only language servers list ordering for multi-language cases. In general nothing significant will change if we replace those magic numbers with two states - match or not match. The problem is aggravated by the fact that matching takes into account only paths and languages, not respecting feature (capability) support level, so it is quite possible to have a big matching score with rather poor functionality, seems not to be what we need.

TextDocumentService does not really uses language server matching for all its manipulations. While language servers comes sorted according to their match score, they are in most cases ran in parallel, which neutralizes sorting effect. Beside that it is TextDocumentService exactly the place where we would better benefit of specific feature support scores but not general language server scores.

Finally, this matching approach does not fit quite well new simplified language server matching algorithms, because all those language ids, extensions, names, paths are to be replaced with regexes.

dkuleshov · 2018-04-12T09:12:29Z

ci-build

codenvy-ci · 2018-04-12T10:41:05Z

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

dkuleshov · 2018-04-12T11:42:47Z

ci-test

codenvy-ci · 2018-04-12T17:42:47Z

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

musienko-maxim · 2018-04-16T13:57:22Z

All tests except CSharpLanguageTest - passed

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

dkuleshov · 2018-04-16T17:56:02Z

ci-build

codenvy-ci · 2018-04-16T22:45:32Z

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

dkuleshov · 2018-04-17T07:08:18Z

ci-test

gazarenkov

LGTM
Let's consider as an LSP support next steps:

retiring Launcher mechanism in favor of workspace configuration for particular (potentially third party) LS
Adding simplified config option for language selecting like option to use extension/file_name instead of regexp (I am not talking about removing regexp option, it may be useful for some advanced cases)

ghost · 2018-04-17T13:00:30Z

Regexp for extension looks pretty simple. I mean as for me, *.py and py do not seem very different. But yes, it can be convenient.

As to retiring LS provided via installers and launchers, will there be separate issues 1) to remove those from code and 2) rework current stacks to use compose recipe type 3) make UD LS friendly, so that adding a language server is a matter of choosing it and clicking add button, since current steps are a bit complicated for an ordinary user.

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

tsmaeder · 2018-04-26T03:14:26Z

...main/java/org/eclipse/che/plugin/languageserver/ide/service/LanguageServerServiceClient.java

@@ -76,4 +75,19 @@ public Throwable getCause() {
      }
    };
  }
+
+  private PromiseError getPromiseError() {


"getTimeoutError()"

tsmaeder · 2018-04-26T03:16:13Z

...n/che-plugin-maven-ide/src/main/java/org/eclipse/che/plugin/maven/client/MavenExtension.java

@@ -118,7 +118,7 @@ private void registerFileType(
      FileTypeRegistry fileTypeRegistry,
      MavenResources mavenResources,
      EditorRegistry editorRegistry) {
-    FileType pomFile = new FileType(mavenResources.maven(), null, "pom\\.xml");
+    FileType pomFile = new FileType(mavenResources.maven(), null, ".*pom\\.xml");


This would match a file named "foopom.xml", probably not what you want?

tsmaeder · 2018-04-26T03:16:44Z

...-ide/src/main/java/org/eclipse/che/plugin/maven/client/MavenLanguageDescriptionProvider.java

+import javax.inject.Provider;
+import org.eclipse.che.api.languageserver.shared.model.LanguageDescription;
+
+public class MavenLanguageDescriptionProvider implements Provider<LanguageDescription> {


I was wondering why we need a Provider?

tsmaeder · 2018-04-26T03:24:48Z

...rc/main/java/org/eclipse/che/plugin/yaml/server/languageserver/YamlLanguageServerConfig.java

-      LanguageServer server,
-      ServerCapabilities capabilities,
-      String projectPath) {
+  public void onLSPRoxyInitialized(LanguageServerInitializedEvent event) {


If this module uses a sidecar, why do we have a launch script? If not, why do we have a language proxy?

Also, the method handles LanguageServerInitializedEvent, so "onLSPRoxyInitialized" seems a strange name.

If I understood correctly.

In fact org.eclipse.lsp4j.services.LanguageServer fits the definition of proxy quite well , because it acts as an intermediary for requests from a client to actual language server. Anyway I'm sure you know that we use it for both parallel container or installer + launcher, so it does not really matter if we have a launch script or not.

However I agree, to avoid further confusion I will make method name a bit clearer.

tsmaeder · 2018-04-26T03:27:35Z

...languageserver/src/main/java/org/eclipse/che/api/languageserver/DefaultInstanceProvider.java

+    return remoteProxy;
+  }
+
+  private static class InstanceHolder {


What is the purpose of this class?

One of the most common implementations of a singleton pattern - Bill Pugh solution

Which provides lazy initialization, which is totally unnecessary in this case. It just adds complication.

tsmaeder · 2018-04-26T03:28:24Z

...guageserver/src/main/java/org/eclipse/che/api/languageserver/EmptyCommunicationProvider.java

+ * provider and the language server server that runs in the same virtual machine and does not use
+ * input/output streams for communication.
+ */
+public class EmptyCommunicationProvider implements CommunicationProvider {


Is this a "null object"?

tsmaeder · 2018-04-26T03:32:00Z

...anguageserver/src/main/java/org/eclipse/che/api/languageserver/InitializeParamsProvider.java

+  }
+
+  /** Provides client capabilities */
+  private static class ClientCapabilitiesProvider {


Seriously, what's with the providers? Just make this a static variable in InitializeParamsProvider

That is a very interesting suggestion, however I tend to disagree and let me please explain why.

InitializeParamsProvider itself is quite big so decomposition here is something that does not require an explanation. However in what way we actually decompose is a very interesting question. In this matter I follow Grady Booch and his book "Object-oriented Analysis and Design" who says that algorithmic decomposition is a necessary part of object-oriented analysis and design, but object-oriented systems start with and emphasize decomposition into classes. In other words I prefer to split the logic into classes, not into fields or variables, but your suggestion is more of a procedural programming.

Yes, but you're not doing algorithmic decomposition, you're just taking a constant and making it more complicated.

Yes, but you're not doing algorithmic decomposition, you're just taking a constant and making it more complicated.

That is not true.

I correct myself: you're taking a simple static method and making it more complicated. Algorighmic decomposition is decomposition into functions, not classes. I heartily disagree with this style.

tsmaeder · 2018-04-26T03:39:15Z

...ageserver/src/main/java/org/eclipse/che/api/languageserver/ProcessCommunicationProvider.java

+  private final String languageServerName;
+  private final StatusChecker statusChecker;
+
+  private Process process;


I don't think this variable is thread safe.

Me neither.

tsmaeder · 2018-04-26T03:55:56Z

If you look at the problem this is supposed to fix, it seems relatively straightforward to fix the config mechanism. Instead, this turned into a 147 file rewrite of the language server registration.
Since this is basically a rewrite, it's really hard to review the changes in a PR, files being moved and renamed.
This PR will generate a significant load on the languages team with respect to the jdt.ls work and therefore I can't put my approval on it.

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

tsmaeder · 2018-04-27T06:38:13Z

...pi-languageserver/src/main/java/org/eclipse/che/api/languageserver/LanguageServerConfig.java

+   *
+   * @return regular expression provider
+   */
+  RegexProvider getRegexpProvider();


As we have discussed on calls before, I don't think replacing the kind of standard "LanguageDescription" with regexes here is something we want to do.

I see a statement, I don't see any argument. No logical reasoning.

tsmaeder · 2018-04-27T06:42:00Z

...pi-languageserver/src/main/java/org/eclipse/che/api/languageserver/LanguageServerConfig.java

+ * @see LanguageServerConfigProvider
+ * @see LanguageServerConfigInitializer
+ */
+public interface LanguageServerConfig {


Generally, I think this interface is way too complicated, as far as the client is concerned all it needs it is a way to get an instance of a language server and perhaps a way to monitor the status of this instance. The old interface nicely captured this. The fact that you're mandating the use of streams ("CommunicationProvider") in the interface is getting an implementation detail into the interface and unnecessarily restricts the implementation.
The fact that we have null implementations and null values (for example the CommunicationProvider) is a code smell that indicates we're describing an implementation, not an interface.

Generally, I think this interface is way too complicated, as far as the client is concerned all it needs it is a way to get an instance of a language server and perhaps a way to monitor the status of this instance. The old interface nicely captured this. The fact that you're mandating the use of streams ("CommunicationProvider") in the interface is getting an implementation detail into the interface and unnecessarily restricts the implementation.

The overcomplicating is caused by the need of backward compatibility with the installers, which is expected to be removed in future, then the configuration become simpler, ComplicationProvider and all implementation dependent stuff will be removed.

The fact that we have null implementations and null values (for example the CommunicationProvider) is a code smell that indicates we're describing an implementation, not an interface.

The fact that we have null implementation of InstanceProvider is the result of a short-sighted implementation of the MavenLanguageServer and the necessity to keep backward compatibility, because it is neither an installed language server, nor a parallel container language server. Again CommunicationProvider null implementation is needed to keep backward compatibility with launchers.

tsmaeder · 2018-04-27T06:45:02Z

...pi-languageserver/src/main/java/org/eclipse/che/api/languageserver/LanguageServerConfig.java

+   * @return <code>null</code>if a language server is not expected to be launched locally, instance
+   *     of an installer status provider otherwise
+   */
+  default InstallerStatusProvider getInstallerStatusProvider() {


We're trying to get away from installers. Why is this in the interface here? Implementation detail.

Backward compatibility.

What kind of backward compatibility? Compile time? Runtime?

tsmaeder · 2018-04-27T06:46:55Z

...pi-languageserver/src/main/java/org/eclipse/che/api/languageserver/LanguageServerConfig.java

+    /**
+     * The implementation must provide aliveness status for language server that we communicate to.
+     */
+    interface StatusChecker {


This interface could be simpler as
String isAlive() where a null return signals "isAlive". Also, right now it's not clear whether the implementer is allowed to return null if "isAlive()" returns true, so you have to check for null anyway.

In object-oriented programming we're trying to keep things simple, clear and declarative way. If the status is "alive" we return true when they ask "is it alive". Shorter code is a bad replacement for a more transparent code.

I guess we'll have to disagree on that one. Anyway, you could at least clarify the javadoc so clients don't have to check for null.

tsmaeder · 2018-04-27T06:47:35Z

...pi-languageserver/src/main/java/org/eclipse/che/api/languageserver/LanguageServerConfig.java

+  }
+
+  /** Provides language server matching regular expression definitions. */
+  interface RegexProvider {


This interface is called "RegexProvider", but provides file watch patterns, as well. This is unexpected.

That's probably because watch patterns are represented by regular expressions as well, unless you can suggest a better name?

I would fold it into the language server description. If something does not have a good name, it's usually not a "thing".

tsmaeder · 2018-04-27T12:48:50Z

...nguageserver/src/main/java/org/eclipse/che/api/languageserver/LanguageServerInitializer.java

+
+        LanguageServer languageServer = languageServerRegistry.get(id);
+        InitializeParams initializeParams = initializeParamsProvider.get(id);
+        InitializeResult initializeResult =


The timeout is very likely too short for jdt.ls.

tsmaeder · 2018-04-27T12:52:28Z

...pi-languageserver/src/main/java/org/eclipse/che/api/languageserver/LanguageServiceUtils.java

@@ -28,19 +28,19 @@ public static String removeUriScheme(String uri) {
    return uri.startsWith(FILE_PROJECTS) ? uri.substring("file://".length()) : uri;
  }

-  public static boolean truish(Boolean b) {
+  static boolean truish(Boolean b) {


I find these could be useful outside of the package.

Agree, might be quite possible, we need to move it somewhere in common modules.

I'm thinking where it should be moved, do you have any idea?

no, that's why I left them here, but public.

tsmaeder · 2018-04-27T12:57:47Z

...geserver/src/main/java/org/eclipse/che/api/languageserver/ServerCapabilitiesAccumulator.java

@@ -0,0 +1,218 @@
+/*


Is this more or less a copy of the preexisting code or did you change something?

Thomas, it is mostly a copy of your class (according to git history) ServerCapabilitiesOverlay.

I made some fixes as well, in master we have

if (leftOptions != null && leftOptions.isResolveProvider() || rightOptions != null && leftOptions.isResolveProvider()) { result.setResolveProvider(true); }

but should be

if (leftOptions != null && leftOptions.isResolveProvider() || rightOptions != null && rightOptions.isResolveProvider()) { result.setResolveProvider(true); }

and in several other places if I remember well. Why don't you ask directly? I guess you are worried that you're not mentioned as the author in javadoc, note that in ServerCapabilitiesOverlay you are not mentioned as the author in javadoc as well, but no worries I will add, my apologies.

No, sine the class has been moved, it's hard to review the change. If it's just a copy, I don't have to go through the effort.

tsmaeder · 2018-04-27T12:59:34Z

...api-languageserver/src/main/java/org/eclipse/che/api/languageserver/TextDocumentService.java

@@ -0,0 +1,769 @@
+/*


I assume this is just a move of the file?

Not exactly, there were several changes according to a new API. Again, I'm sorry for not adding you as an author, the fix is coming.

Again, I don't care about that, only about the correctness of the code.

tsmaeder · 2018-04-27T13:01:10Z

...languageserver/src/main/java/org/eclipse/che/api/languageserver/WorkspaceConfigProvider.java

+    Runtime runtime = workspace.getRuntime();
+    if (runtime == null) {
+      return ImmutableMap.of();
+    }


Are these two not error conditions that should be logged?

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

tsmaeder · 2018-05-02T09:19:37Z

...nguageserver/src/main/java/org/eclipse/che/api/languageserver/LanguageServerInitializer.java

+    this.serverCapabilitiesAccumulator = serverCapabilitiesAccumulator;
+    this.findId = findId;
+
+    this.idRegistry = registryContainer.idRegistry;


So the "idRegistry" is really a form of String interning to ensure that we get the same instance of the String for locking, correct? That deserves a comment, IMO.

dkuleshov · 2018-05-14T10:38:39Z

ci-build

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

I don't think we're going to agree on the design.

dkuleshov · 2018-05-14T13:08:35Z

ci-test

codenvy-ci · 2018-05-14T14:31:48Z

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

codenvy-ci · 2018-05-14T18:31:54Z

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

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

musienko-maxim · 2018-05-17T14:03:28Z

ci-test

codenvy-ci · 2018-05-17T19:02:14Z

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

musienko-maxim · 2018-05-18T09:15:00Z

The existed selenium tests did not find new regression.
Moreover we checked all language servers according the test plan manually: https://github.com/eclipse/che/wiki/Test-plan-for-checking-Language-servers
Also was checked feature with regexp on TypeScript LS. under 'sidecar'.
During test session all bugs was fixed and after that we did not find new bugs.
The problem with C# LS. and go still actual. More details here: #9351

ElectronicWizzard · 2018-06-22T10:21:49Z

I am trying to get my custom LSP integrated as sidecar using Che 6.7.0 by configuration.
Is the parameter option "languageRegexes":"[ {"languageId":"c", "regex":".\\.(c|h)"}, {"languageId":"cpp", "regex":".\\.(cpp|hh)"}]" already implemented and working or is this still work in progress?

dkuleshov · 2018-06-23T09:14:11Z

Hi @ElectronicWizzard

It has been introduced in 6.6.0. However looking at your example I'd say that the correct configuration should probably be more similar to this:

"languageRegexes":"[ {\"languageId\":\"c\", \"regex\":\".\\\\.(c|h)$\"}, {\"languageId\":\"cpp\", \"regex\":\".\\\\.(cpp|hh)$\"}]"

Perhaps @eivantsov may assist you more on your purposes as he is an expert in language server configuration and related things.

ElectronicWizzard · 2018-06-25T17:30:36Z

@eivantsov - I am looking for a E2E documentation. I tried now various options but language server is not started once a click on a file with the defined extension in my workspace. I do not see any log information in the dev maschine
I followed the setup described under https://www.eclipse.org/che/docs/language-servers.html#ls-sidecars.
Here is my workspace config including my custom ls in a own image.
{
"environments": {
"default": {
"recipe": {
"contentType": "text/x-yaml",
"type": "compose",
"content": "services:\n cdx-ls-machine:\n image: '127.0.0.1:5000/cdxlsp'\n mem_limit: 1073741824\n dev-machine:\n image: '127.0.0.1:5000/checap'\n mem_limit: 2147483648\n depends_on:\n - cdx-ls-machine\n"
},
"machines": {
"dev-machine": {
"env": {
"CHE_MACHINE_NAME": "dev-machine"
},
"installers": [
"org.eclipse.che.exec",
"org.eclipse.che.terminal",
"org.eclipse.che.ws-agent",
"org.eclipse.che.ls.yaml"
],
"volumes": {
"projects": {
"path": "/projects"
}
},
"servers": {},
"attributes": {
"memoryLimitBytes": "2147483648"
}
},
"cdx-ls-machine": {
"env": {
"CHE_MACHINE_NAME": "cdx-ls-machine"
},
"installers": [],
"volumes": {
"projects": {
"path": "/projects"
}
},
"servers": {
"ls": {
"attributes": {
"languageRegexes": "[{"languageId":"cdx", "regex":".\\.(cdx)$"}]",
"internal": "true",
"id": "cdx",
"type": "ls"
},
"protocol": "tcp",
"port": "4417"
}
},
"attributes": {
"memoryLimitBytes": "2147483648"
}
}
}
}
},
"defaultEnv": "default",
"commands": [],
"projects": [],
"name": "caplsp",
"links": []
}

ghost · 2018-06-25T17:41:18Z

@dkuleshov this won't work for a language id that is not in the list of predefined ones, will it?

dkuleshov · 2018-06-26T11:17:31Z

@eivantsov as far as I know should work irrespectively of language id definition

…ured through workspace configuration (eclipse-che#9387)

dkuleshov added the status/in-progress This issue has been taken by an engineer and is under active development. label Apr 12, 2018

dkuleshov self-assigned this Apr 12, 2018

dkuleshov requested review from svor, azatsarynnyy, vparfonov, tolusha, vzhukovs, RomanNikitenko and vinokurig April 12, 2018 09:12

dkuleshov requested a review from evidolob as a code owner April 12, 2018 09:12

dkuleshov requested a review from gazarenkov April 12, 2018 09:15

vparfonov changed the title ~~Several improvements to the way remote language servers may be configured through workspace configuration~~ [WIP] Several improvements to the way remote language servers may be configured through workspace configuration Apr 12, 2018

dkuleshov added team/ide kind/enhancement A feature request - must adhere to the feature request template. labels Apr 12, 2018

dkuleshov requested review from tsmaeder and a user April 16, 2018 09:31

initial commit

4714d6f

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

dkuleshov force-pushed the che#8925 branch from 217f5db to 4714d6f Compare April 16, 2018 17:55

dkuleshov requested a review from skabashnyuk as a code owner April 16, 2018 17:55

vparfonov mentioned this pull request Apr 17, 2018

IDE-2018-05-08 (Sprint: 148) #9411

Closed

20 tasks

gazarenkov approved these changes Apr 17, 2018

View reviewed changes

even more very important fixes

86698e5

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

tolusha approved these changes Apr 25, 2018

View reviewed changes

tsmaeder reviewed Apr 26, 2018

View reviewed changes

fresh minor fixes

b5b26f4

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

tsmaeder previously requested changes Apr 27, 2018

View reviewed changes

Dmytro Kulieshov added 2 commits April 28, 2018 13:59

applied @tsmaeder remarks

9273f88

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

fixed javadocs

49e3fbd

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

tsmaeder reviewed May 2, 2018

View reviewed changes

vparfonov mentioned this pull request May 5, 2018

IDE-2018-05-29 (Sprint: 149) #9628

Closed

21 tasks

Merge branch 'master' into che#8925

0e745b2

corrected visibility of some methods in LanguageServerUtils, authors

6f397ec

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

fixed language description initialization

ee1c680

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

dkuleshov merged commit 7a52c83 into master May 18, 2018

dkuleshov deleted the che#8925 branch May 18, 2018 09:17

benoitf added this to the 6.6.0 milestone May 18, 2018

hbhargav pushed a commit to hbhargav/che that referenced this pull request Dec 5, 2018

Several improvements to the way remote language servers may be config…

bb0b4a7

…ured through workspace configuration (eclipse-che#9387)

[WIP] Several improvements to the way remote language servers may be configured through workspace configuration #9387

[WIP] Several improvements to the way remote language servers may be configured through workspace configuration #9387

Conversation

dkuleshov commented Apr 12, 2018 • edited Loading

Workspace language server configuration

Guice language server configuration

Language descriptions

Language server matching

dkuleshov commented Apr 12, 2018

codenvy-ci commented Apr 12, 2018

dkuleshov commented Apr 12, 2018

codenvy-ci commented Apr 12, 2018

musienko-maxim commented Apr 16, 2018

dkuleshov commented Apr 16, 2018

codenvy-ci commented Apr 16, 2018

dkuleshov commented Apr 17, 2018

gazarenkov left a comment

Choose a reason for hiding this comment

ghost commented Apr 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkuleshov Apr 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsmaeder Apr 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsmaeder commented Apr 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkuleshov Apr 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkuleshov Apr 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkuleshov Apr 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkuleshov commented May 14, 2018

dkuleshov commented May 14, 2018

codenvy-ci commented May 14, 2018

codenvy-ci commented May 14, 2018

musienko-maxim commented May 17, 2018

codenvy-ci commented May 17, 2018

musienko-maxim commented May 18, 2018

ElectronicWizzard commented Jun 22, 2018

dkuleshov commented Jun 23, 2018 • edited Loading

ElectronicWizzard commented Jun 25, 2018

dkuleshov commented Apr 12, 2018 •

edited

Loading

dkuleshov Apr 27, 2018 •

edited

Loading

tsmaeder Apr 26, 2018 •

edited

Loading

dkuleshov Apr 27, 2018 •

edited

Loading

dkuleshov Apr 27, 2018 •

edited

Loading

dkuleshov Apr 27, 2018 •

edited

Loading

dkuleshov commented Jun 23, 2018 •

edited

Loading