GH-1845: Extended the LSP with the semantic highlighting capabilities. #2332

kittaakos · 2018-07-10T18:35:47Z

Signed-off-by: Akos Kitta kittaakos@typefox.io

akosyakov · 2018-07-11T05:43:22Z

packages/core/src/common/disposable.ts

    protected readonly disposables: Disposable[] = [];
    protected readonly onDisposeEmitter = new Emitter<void>();

+    constructor(toDispose?: Disposable | Disposable[]) {


Nice idea! How about using varargs?

constructor(...disposables: Disposable[]) { this.pushAll(disposables); }

akosyakov · 2018-07-11T05:45:44Z

packages/editor/src/browser/semantic-highlight/semantic-highlighting-service.ts

+}
+
+@injectable()
+export class NoopSemanticHighlightingService implements SemanticHighlightingService {


How about turning SemanticHighlightingService into a class and use it as no-op implementation to get rid of additional symbol, inteface and bindings? Similary to QuickOpenService.

kittaakos · 2018-07-27T12:59:57Z

package.json

@@ -23,6 +23,7 @@
    "chai-string": "^1.4.0",
    "concurrently": "^3.5.0",
    "electron-mocha": "~3.5.0",
+    "esm": "^3.0.72",


TODO: remove this.

kittaakos · 2018-07-27T13:00:33Z

packages/java/src/browser/monaco-contribution/java-textmate-contribution.ts

 import { LanguageGrammarDefinitionContribution, TextmateRegistry } from '@theia/monaco/lib/browser/textmate';

 @injectable()
 export class JavaTextmateContribution implements LanguageGrammarDefinitionContribution {

+    // TODO: restore this!


kittaakos · 2018-07-27T13:01:21Z

packages/monaco/src/browser/monaco-semantic-highlighting-service.ts

+     * - `from` can be greater than `to`. If so, the arguments will be swapped internally and you still get back `[to, to + 1, ..., from -1, from]`
+     * - If `from` and `to` are the same, returns with a single element range: `[from]`.
+     */
+    protected rangeOf(from: number, to: number): number[] {


TODO: remove this.

kittaakos · 2018-07-27T13:02:00Z

packages/monaco/src/browser/monaco-semantic-highlighting-service.ts

+
+}
+
+// /**


TODO: clean this up.

kittaakos · 2018-08-01T11:12:47Z

packages/java/scripts/download-jdt-ls.js

-const downloadURI = packageJson['ls.download.base'] || 'https://www.eclipse.org/downloads/download.php?file=';
-const archiveUri = downloadURI + serverPath + '&r=1';
+// TODO revert this!
+const serverPath = 'https://github.com/kittaakos/eclipse.jdt.ls-gh-715/raw/master/jdt-language-server-latest.tar.gz';//packageJson['ls.download.path'] || '/che/che-ls-jdt/snapshots/che-jdt-language-server-latest.tar.gz';


TODO: It is a hack. Revert this.

svenefftinge · 2018-08-06T07:00:45Z

packages/editor/src/browser/semantic-highlight/semantic-highlighting-service.ts

+import { Disposable } from '@theia/core/lib/common/disposable';
+
+@injectable()
+export class SemanticHighlightingService implements Disposable {


We should add some comment, explaining why this impl is entirely empty. It's ok to have a pointer to the monaco implementation in it.

I can move the entire implementation from @theia/monaco to @theia/editor if I can expose the id property for the editor decoration.

The API for the EditorDecoration lacks this information.

svenefftinge · 2018-08-06T07:13:23Z

packages/monaco/src/browser/monaco-semantic-highlighting-service.ts

+    }
+
+    /**
+     * We do not get delta notification from the LS if lines were deleted and new semantic highlighting positions were introduced.


I remember we talked about it, but I forget why that is. I think it should be the responsibility of the LS to send updates in that case, too.

svenefftinge · 2018-08-06T07:14:42Z

packages/editor/src/browser/editor.ts

+     * @param id The decoration id.
+     * @return The decoration range or `undefined` if the decoration was not found.
+     */
+    getDecorationRange(id: string): Range | undefined;


I couldn't find any clients for this.

svenefftinge

Cool! Looks very good to me :)
I noticed that neither in Java neither static members were italic (didn't notice a difference) nor were deprecated underlined (they are turned red and italic)

svenefftinge · 2018-08-06T07:18:57Z

Ah, no sorry, static fields are italic but methods not.

kittaakos · 2018-08-06T07:22:29Z

Ah, no sorry, static fields are italic but methods not.

Static method declarations are not, static method invocations are italic. Just like in Eclipse with JDT.

kittaakos · 2018-08-07T13:17:28Z

@tsmaeder, what is the difference between the https://www.eclipse.org/downloads/download.php?file=/che/che-ls-jdt/snapshots/che-jdt-language-server-latest.tar.gz and
http://download.eclipse.org/jdtls/snapshots/jdt-language-server-latest.tar.gz LSs? Thanks!

svenefftinge

LGTM

kittaakos · 2018-08-07T14:21:21Z

I would like to check the Windows failure before merging this.

kittaakos · 2018-08-07T15:32:48Z

Bumping the test timeout fixed the issue. I do one more squash and build, then I merge this.

Closes #1845 Signed-off-by: Akos Kitta <kittaakos@typefox.io>

akosyakov · 2018-08-08T08:43:41Z

packages/java/src/node/java-cli-contribution.ts

+
+import { injectable } from 'inversify';
+import { Argv, Arguments } from 'yargs';
+import { CliContribution } from '@theia/core/src/node/cli';


src -> lib

akosyakov reviewed Jul 11, 2018

View reviewed changes

kittaakos force-pushed the GH-1845 branch 3 times, most recently from e1a1818 to 71009b7 Compare July 11, 2018 07:36

kittaakos force-pushed the GH-1845 branch from 71009b7 to 3419c3f Compare July 18, 2018 11:45

kittaakos force-pushed the GH-1845 branch from e88ed90 to 42dc535 Compare July 27, 2018 12:59

kittaakos commented Jul 27, 2018

View reviewed changes

packages/monaco/src/browser/monaco-semantic-highlighting-service.ts Outdated

}

// /**

Copy link

Contributor Author

kittaakos Jul 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: clean this up.

kittaakos force-pushed the GH-1845 branch 2 times, most recently from 18450e9 to ceb74d2 Compare August 1, 2018 11:11

kittaakos commented Aug 1, 2018

View reviewed changes

svenefftinge reviewed Aug 6, 2018

View reviewed changes

kittaakos force-pushed the GH-1845 branch 4 times, most recently from bb86112 to c1f5c6d Compare August 6, 2018 17:03

kittaakos changed the title ~~[WIP] GH-1845: Extended the LSP with the semantic highlighting capabilities.~~ GH-1845: Extended the LSP with the semantic highlighting capabilities. Aug 7, 2018

This was referenced Aug 7, 2018

Semantic highlighting protocol extension microsoft/vscode-languageserver-node#368

Closed

Proposal of the semantic highlighting protocol extension microsoft/vscode-languageserver-node#367

Closed

kittaakos force-pushed the GH-1845 branch 2 times, most recently from 5bd6956 to 0fb4300 Compare August 7, 2018 12:52

svenefftinge approved these changes Aug 7, 2018

View reviewed changes

GH-1845: Extended the LSP with the semantic highlighting capabilities.

f25a622

Closes #1845 Signed-off-by: Akos Kitta <kittaakos@typefox.io>

kittaakos force-pushed the GH-1845 branch from 29ef995 to f25a622 Compare August 7, 2018 15:33

kittaakos merged commit cda47a7 into master Aug 8, 2018

kittaakos deleted the GH-1845 branch August 8, 2018 06:47

akosyakov reviewed Aug 8, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-1845: Extended the LSP with the semantic highlighting capabilities. #2332

GH-1845: Extended the LSP with the semantic highlighting capabilities. #2332

kittaakos commented Jul 10, 2018 •

edited

Loading

akosyakov Jul 11, 2018

akosyakov Jul 11, 2018

kittaakos Jul 27, 2018

kittaakos Jul 27, 2018

kittaakos Jul 27, 2018

kittaakos Jul 27, 2018

kittaakos Aug 1, 2018

kittaakos Aug 6, 2018

svenefftinge Aug 6, 2018

kittaakos Aug 6, 2018

svenefftinge Aug 6, 2018

svenefftinge Aug 6, 2018

svenefftinge left a comment

svenefftinge commented Aug 6, 2018

kittaakos commented Aug 6, 2018

kittaakos commented Aug 7, 2018

svenefftinge left a comment

kittaakos commented Aug 7, 2018

kittaakos commented Aug 7, 2018

akosyakov Aug 8, 2018

GH-1845: Extended the LSP with the semantic highlighting capabilities. #2332

GH-1845: Extended the LSP with the semantic highlighting capabilities. #2332

Conversation

kittaakos commented Jul 10, 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

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

svenefftinge left a comment

Choose a reason for hiding this comment

svenefftinge commented Aug 6, 2018

kittaakos commented Aug 6, 2018

kittaakos commented Aug 7, 2018

svenefftinge left a comment

Choose a reason for hiding this comment

kittaakos commented Aug 7, 2018

kittaakos commented Aug 7, 2018

Choose a reason for hiding this comment

kittaakos commented Jul 10, 2018 •

edited

Loading