Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the textDocument/callHierarchy extension #520

Closed
MaskRay opened this issue Sep 13, 2019 · 8 comments
Closed

Improve the textDocument/callHierarchy extension #520

MaskRay opened this issue Sep 13, 2019 · 8 comments
Assignees
Milestone

Comments

@MaskRay
Copy link

MaskRay commented Sep 13, 2019

Requested to create a new issue. I haven't looked closely to the latest updates.

Pasted from #420 (comment)

I think the proposal has several deficiency that require further improvement. Comments inlined below:

export interface CallHierarchyCall {
  // If there are multiple `CallHierarchyCall` elements with the same `from`, will the `from: CallHierarchySymbol` be duplicated multiple times?
  range: Range;
  from: CallHierarchySymbol;
  to: CallHierarchySymbol;
}
result: CallHierarchyCall[] | null

export interface CallHierarchySymbol {
  // There is no id field. `name` or `detail` may not identify a specific target that can be suitable for further requests (e.g. node expansion).
  name: string;
  detail?: string;
  kind: SymbolKind;
  uri: string;
  range: Range;
  selectionRange: Range;
}

In ccls (https://github.com/MaskRay/ccls/blob/master/src/messages/ccls_call.cc), we use:

struct Out_cclsCall {
  Usr usr;  // language server specific
  std::string id;
  std::string_view name;
  Location location;
  CallType callType = CallType::Direct;
  int numChildren;
  // Empty if the |levels| limit is reached.
  std::vector<Out_cclsCall> children;
};

The method $ccls/call was designed so that the result can either be a hierarchy or a flattened list (like textDocument/references).

@dbaeumer
Copy link
Member

@jrieken @aeschli FYI.

@jrieken
Copy link
Member

jrieken commented Sep 30, 2019

We have discussed to model the call hierarchy as object graph and decided against it because it raises questions about loops, recursion, and depth. Instead we went with retrieving nodes only (and repeatedly). The latest proposal (which addresses some of your feedback) is here: microsoft/vscode#70231 (comment)

@dbaeumer dbaeumer added this to the 3.16 milestone Oct 24, 2019
@KamasamaK
Copy link

KamasamaK commented Oct 28, 2019

The VS Code API has been finalized.

@kjeremy
Copy link
Contributor

kjeremy commented Apr 15, 2020

Since the VS Code API has been finalized can the LSP side be finalized?

@dbaeumer
Copy link
Member

Yes. A PR would currently speed this up :-).

@kjeremy
Copy link
Contributor

kjeremy commented Apr 16, 2020

@dbaeumer I don't see any steps for moving something out of the proposed state. Is that mechanically just moving everything out of proposed and updating the md file?

@dbaeumer see #614

@dbaeumer dbaeumer self-assigned this Apr 28, 2020
@dbaeumer
Copy link
Member

dbaeumer commented May 6, 2020

I merge the PR and did some cleanup work and added it to the spec.

@dbaeumer dbaeumer closed this as completed May 6, 2020
@dbaeumer
Copy link
Member

dbaeumer commented May 7, 2020

Closing the issue since the PR is merged.

@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants