GH-707: Introduced the `@theia/typehierarchy` extension. #3802

kittaakos · 2018-12-11T07:44:44Z

Originally from here: #3148.

Extended the LSP with the type hierarchy.
From now on, a tree node can carry decoration data.
Added editor access for the current/active editors.

Closes: #707.

Signed-off-by: Akos Kitta [email protected]

simark · 2018-12-14T17:21:53Z

In #3148, you mentioned that this implementation differs from the proposal in microsoft/vscode-languageserver-node#346. Can you explain how it differs?

kittaakos · 2018-12-14T17:52:10Z

#3148 (comment)

simark · 2018-12-14T18:10:34Z

I saw this, but I don't really understand what to get from it.

kittaakos · 2018-12-14T18:19:36Z

I don't really understand

Which part needs clarification?

simark · 2018-12-14T18:22:18Z

Well, what in your implementation differs from the proposal. Different methods, fields?

kittaakos · 2018-12-14T18:36:32Z

I see. The links are broken so you cannot compare the implementation with the proposed protocol changes; so I cannot do this either right now :(

By the way, we most likely throw away the LSP related changes from the PR and reimplement it based on this. Knowing these, are the differences still relevant to you?

simark · 2018-12-14T18:44:18Z

By the way, we most likely throw away the LSP related changes from the PR and reimplement it based on this. Knowing these, are the differences still relevant to you?

In any case, I am just interested to know what this patch implements. I think it would be relevant to mention and explain it in the commit message. If you implemented the version in the comment you linked, you can mention that. If you made changes compared to it, it would be good to mention what they are as well.

HighCommander4 · 2019-01-09T00:19:19Z

I implemented server support for the superTypes part of this in clangd (C++ language server) in this patch, and wrote a patch to enable it in Theia's C++ language client.

One question, @kittaakos: why does this need to be enabled separately for each language client (e.g. separately in java-client-contribution.ts vs. cpp-client-contribution.ts? I understand that not every language has the concept of super- and subtypes, and we don't want to show menu items that aren't applicable to the language, but isn't the "capabilities" mechanism sufficient to take care of this? That is, a server for a language that doesn't have a concept of super- and subtypes would not include typeHierarchy in ServerCapabilities, and thus Theia wouldn't show the menus anyways.

kittaakos · 2019-01-21T08:48:29Z

why does this need to be enabled separately for each language client (e.g. separately in java-client-contribution.ts vs. cpp-client-contribution.ts?

@HighCommander4, thank you for your remark. 👍 This PR is a POC. Once the type hierarchy LSP extension is accepted, we can register the feature in a generic way by relying on the server capabilities. Hence, the explicit type hierarchy feature registration per language will not be necessary.

Please note, the proposal and the reference implementation will change due to this: microsoft/vscode-languageserver-node#346 (comment)

HighCommander4 · 2019-01-27T06:41:09Z

packages/languages/src/browser/typehierarchy/typehierarchy-protocol.ts

+
+    /**
+     * The range that should be selected and revealed when this type hierarchy item is being picked, e.g the name
+     * of a function. Must be contained by the the `range`.


Typo: duplicate "the"

HighCommander4 · 2019-01-27T06:44:15Z

packages/languages/src/browser/typehierarchy/typehierarchy-protocol.ts

+export interface TypeHierarchyParams extends TextDocumentPositionParams {
+
+    /**
+     * The hierarchy levels to resolve. `0` indicates no level. When no defined, it is treated as `0`.


typo: no defined -> not defined

HighCommander4 · 2019-01-27T06:46:55Z

packages/languages/src/browser/typehierarchy/typehierarchy-protocol.ts

+    /**
+     * The item to resolve.
+     */
+    item: TypeHierarchyItem;


Why does the client need to send a full TypeHierarchyItem - which includes things like the symbol kind, range, and selection range - to identify what it wants to resolve?

To have a symmetric API with other existing language feature. For instance, completionItem/resolve.

HighCommander4 · 2019-01-27T07:15:51Z

packages/languages/src/browser/typehierarchy/typehierarchy-protocol.ts

+    item: TypeHierarchyItem;
+
+    /**
+     * The hierarchy levels to resolve. `0` indicates no level.


Should there be a way of saying "resolve all levels"?

You can use Number.MAX_SAFE_INTEGER.

kittaakos · 2019-01-28T09:05:27Z

this implementation differs from the proposal

It is not the case anymore.

AlexTugarev · 2019-02-01T16:57:32Z

@kittaakos, FYI [email protected] implements this protocol extension.

HighCommander4 · 2019-02-03T23:48:57Z

packages/typehierarchy/src/browser/tree/typehierarchy-tree.ts

+                    node.resolved = true;
+                    const items = TypeHierarchyType.SUBTYPE === type ? resolvedItem.children : resolvedItem.parents;
+                    if (items) {
+                        node.children = items.map(child => TypeHierarchyTree.Node.create(child, type));


Should this be passing resolved = false to create()? The protocol call above is using resolve: 1, so the node itself will be resolved but the children won't be.

vinayb21 · 2019-02-04T06:37:02Z

tsconfig.json

@@ -121,4 +124,4 @@
    "packages/*/src",
    "examples/*/test"
  ]
-}
+}


Add new line at the end.

kittaakos · 2019-03-05T10:29:06Z

Ready for the review.

This PR works with a forked JDT LS that has been rebased from eclipse-jdtls/eclipse.jdt.ls@ec52798

kittaakos · 2019-03-05T20:45:38Z

packages/java/src/browser/java-client-contribution.ts

-        client.registerFeature(SemanticHighlightingService.createNewFeature(this.semanticHighlightingService, client));
+        client.registerFeatures([
+            SemanticHighlightingService.createNewFeature(this.semanticHighlightingService, client),
+        ]);


kittaakos · 2019-03-05T20:46:09Z

packages/languages/src/browser/semantic-highlighting/semantic-highlighting-feature.ts

@@ -24,7 +24,7 @@ import {
    ServerCapabilities,
    Disposable,
    DocumentSelector
-} from '../';
+} from '../index';


kittaakos · 2019-03-05T20:46:42Z

packages/languages/src/browser/typehierarchy/typehierarchy-feature.ts

+/**
+ * Enumeration of available type hierarchy types.
+ */
+export enum TypeHierarchyType {


kittaakos · 2019-03-05T20:47:28Z

packages/typehierarchy/package.json

+    "@theia/core": "^0.4.0",
+    "@theia/editor": "^0.4.0",
+    "@theia/languages": "^0.4.0",
+    "@theia/monaco": "^0.4.0",


kittaakos · 2019-03-05T20:49:17Z

packages/typehierarchy/src/browser/typehierarchy-service.ts

+    /**
+     * Performs the `textDocument/typeHierarchy` LS method invocations.
+     */
+    protected async types(arg: TypeHierarchyItem | TextDocumentPositionParams | Location, direction: TypeHierarchyDirection): Promise<TypeHierarchyItem | undefined> {


todo: public

kittaakos · 2019-03-05T20:50:35Z

packages/typehierarchy/src/browser/typehierarchy-service.ts

+    /**
+     * Converts the argument into a text document position parameter. Returns with the argument if it was a text document position parameter.
+     */
+    protected toTextDocumentPositionParams(arg: TypeHierarchyItem | TextDocumentPositionParams | Location): TypeHierarchyParams {


wrong. should call typeHierarchy/resolve instead.

kittaakos · 2019-03-05T21:04:18Z

we can register the feature in a generic way

It's done.

- Extended the LSP with the type hierarchy. - From now on, a tree node can carry decoration data. - Added editor access for the current/active editors. Closes: #707. Signed-off-by: Akos Kitta <[email protected]>

kittaakos · 2019-03-06T08:34:48Z

Ready for the review. A try-out branch is here: #4490

AlexTugarev

I ❤️ it! I tried it with the test PR.

Just two remarks, which can be discussed and tackled separately.

The menu contribution: WDYT of adding an Code Analysis sub-menu to the context menu? I fear that it will "explode" with the call hierarchy contribution.
A progress indicator would be nice, as some request might take a while, e.g. here the subtypes for Runnable, which comes back after +10 seconds and in the meantime the view is empty.

@nridge

Summary: Patch by Nathan Ridge(@nridge)! This is an LSP extension proposed here: microsoft/vscode-languageserver-node#426 An example client implementation can be found here: eclipse-theia/theia#3802 Reviewers: kadircet, sammccall Reviewed By: kadircet Subscribers: jdoerfert, sammccall, cfe-commits, mgorny, dschaefer, simark, ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet Tags: #clang Differential Revision: https://reviews.llvm.org/D56370 llvm-svn: 356445

@nridge

Summary: Patch by Nathan Ridge(@nridge)! This is an LSP extension proposed here: microsoft/vscode-languageserver-node#426 An example client implementation can be found here: eclipse-theia/theia#3802 Reviewers: kadircet, sammccall Reviewed By: kadircet Subscribers: jdoerfert, sammccall, cfe-commits, mgorny, dschaefer, simark, ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet Tags: #clang Differential Revision: https://reviews.llvm.org/D56370 git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@356445 91177308-0d34-0410-b5e6-96231b3b80d8

@nridge

Summary: Patch by Nathan Ridge(@nridge)! This is an LSP extension proposed here: microsoft/vscode-languageserver-node#426 An example client implementation can be found here: eclipse-theia/theia#3802 Reviewers: kadircet, sammccall Reviewed By: kadircet Subscribers: jdoerfert, sammccall, cfe-commits, mgorny, dschaefer, simark, ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet Tags: #clang Differential Revision: https://reviews.llvm.org/D56370 llvm-svn: 356445

@nridge

Summary: Patch by Nathan Ridge(@nridge)! This is an LSP extension proposed here: microsoft/vscode-languageserver-node#426 An example client implementation can be found here: eclipse-theia/theia#3802 Reviewers: kadircet, sammccall Reviewed By: kadircet Subscribers: jdoerfert, sammccall, cfe-commits, mgorny, dschaefer, simark, ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet Tags: #clang Differential Revision: https://reviews.llvm.org/D56370 llvm-svn: 356445

kittaakos mentioned this pull request Dec 11, 2018

GH-707: Introduced the @theia/typehierarchy extension. #3148

Closed

kittaakos force-pushed the GH-707 branch 3 times, most recently from 380dd03 to 1863194 Compare January 25, 2019 15:21

HighCommander4 reviewed Jan 28, 2019

View reviewed changes

kittaakos force-pushed the GH-707 branch from 1863194 to e99b655 Compare January 28, 2019 08:57

HighCommander4 reviewed Feb 3, 2019

View reviewed changes

vinayb21 reviewed Feb 4, 2019

View reviewed changes

kittaakos force-pushed the GH-707 branch from e99b655 to 6c68603 Compare March 5, 2019 09:20

kittaakos force-pushed the GH-707 branch from 6c68603 to 3b726b6 Compare March 5, 2019 20:43

kittaakos commented Mar 5, 2019

View reviewed changes

kittaakos force-pushed the GH-707 branch from 3b726b6 to 7576218 Compare March 6, 2019 07:32

kittaakos force-pushed the GH-707 branch 2 times, most recently from 79ffbdf to 890755a Compare March 6, 2019 07:47

AlexTugarev self-requested a review March 6, 2019 07:51

GH-707: Introduced the @theia/typehierarchy extension.

1c44854

- Extended the LSP with the type hierarchy. - From now on, a tree node can carry decoration data. - Added editor access for the current/active editors. Closes: #707. Signed-off-by: Akos Kitta <[email protected]>

kittaakos force-pushed the GH-707 branch from 890755a to 1c44854 Compare March 6, 2019 08:24

kittaakos mentioned this pull request Mar 6, 2019

[no-merge] Enabled the type hierarchy for Java and TS. #4490

Closed

AlexTugarev approved these changes Mar 7, 2019

View reviewed changes

kittaakos merged commit 8ff594a into master Mar 7, 2019

marcdumais-work deleted the GH-707 branch March 20, 2019 10:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-707: Introduced the `@theia/typehierarchy` extension. #3802

GH-707: Introduced the `@theia/typehierarchy` extension. #3802

kittaakos commented Dec 11, 2018 •

edited

Loading

simark commented Dec 14, 2018

kittaakos commented Dec 14, 2018

simark commented Dec 14, 2018

kittaakos commented Dec 14, 2018

simark commented Dec 14, 2018

kittaakos commented Dec 14, 2018

simark commented Dec 14, 2018

HighCommander4 commented Jan 9, 2019

kittaakos commented Jan 21, 2019

HighCommander4 Jan 27, 2019

HighCommander4 Jan 27, 2019

HighCommander4 Jan 27, 2019

kittaakos Jan 28, 2019

HighCommander4 Jan 27, 2019

kittaakos Jan 28, 2019

kittaakos commented Jan 28, 2019

AlexTugarev commented Feb 1, 2019

HighCommander4 Feb 3, 2019

vinayb21 Feb 4, 2019

kittaakos commented Mar 5, 2019

kittaakos Mar 5, 2019

kittaakos Mar 5, 2019

kittaakos Mar 5, 2019

kittaakos Mar 5, 2019

kittaakos Mar 5, 2019

kittaakos Mar 5, 2019

kittaakos commented Mar 5, 2019

kittaakos commented Mar 6, 2019

AlexTugarev left a comment

GH-707: Introduced the @theia/typehierarchy extension. #3802

GH-707: Introduced the @theia/typehierarchy extension. #3802

Conversation

kittaakos commented Dec 11, 2018 • edited Loading

simark commented Dec 14, 2018

kittaakos commented Dec 14, 2018

simark commented Dec 14, 2018

kittaakos commented Dec 14, 2018

simark commented Dec 14, 2018

kittaakos commented Dec 14, 2018

simark commented Dec 14, 2018

HighCommander4 commented Jan 9, 2019

kittaakos commented Jan 21, 2019

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

kittaakos commented Jan 28, 2019

AlexTugarev commented Feb 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kittaakos commented Mar 5, 2019

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

kittaakos commented Mar 5, 2019

kittaakos commented Mar 6, 2019

AlexTugarev left a comment

Choose a reason for hiding this comment

GH-707: Introduced the `@theia/typehierarchy` extension. #3802

GH-707: Introduced the `@theia/typehierarchy` extension. #3802

kittaakos commented Dec 11, 2018 •

edited

Loading