emacs-lsp / emacs-ccls

Emacs client for ccls, a C/C++ language server
200 stars 29 forks source link

Rewrite (call|inheritance|member) hierarchy to use lsp-treemacs #74

Closed danielmartin closed 5 months ago

danielmartin commented 4 years ago

This PR removes the ccls-tree named feature that was created ad-hoc for ccls and replaces it with an lsp-treemacs implementation. lsp-treemacs is a package of choice when lsp clients want to display side windows with tree information. Using lsp-treemacs makes for a more homogeneous experience across programming languages in Emacs.

MaskRay commented 4 years ago

Awesome! I've verified that ccls-call-hierarchy ccls-inheritance-hierarchy and ccls-member-hierarchy work. I've got a few questions:

yyoncho commented 4 years ago

When I call ccls-call-hierarchy, the LSP Lookup window does not get focus. How can I make it get the focus?

The original idea of not taking the focus was to benefit from being async. E. g. in lsp-treemacs-references you could do trigger find references call and continue working in the buffer while the results are calculated. Maybe we could revisit that and have

M-x lsp-treemacs-XXX - takes the focus C-u M-x lsp-treemacs-XXX - does not take the focus.

danielmartin commented 4 years ago
M-x lsp-treemacs-XXX - takes the focus
C-u M-x lsp-treemacs-XXX - does not take the focus.

Looks good. I can implement that functionality. We should do the same for the rest of the lsp-treemacs commands.

Is there an ivy-call mode? With the old implementation, when I pressed j or k in the tree window, Emacs jumped to the file:line without exiting tree navigation. With the new implementation, am I supposed to press j RET j RET j RET to navigate 3 items?

I also think the peek functionality is important; I use it a lot from ivy-xref, for example. There's a treemacs-peek command that apparently does something similar to ivy-call, but it only works for files. We would need to extend treemacs to be able to use this peek functionality from LSP. I'm not sure if Treemacs already provides a hook to let us implement treemacs-peek more easily.

The old code expands 2 levels of nodes (:levels 2). The lsp-treemacs implementation does not seem to do so.

This also seems to be a limitation in Treemacs. I think this is less important that the other things, unless I'm missing some use case.

MaskRay commented 4 years ago
M-x lsp-treemacs-XXX - takes the focus
C-u M-x lsp-treemacs-XXX - does not take the focus.

Looks good. I can implement that functionality. We should do the same for the rest of the lsp-treemacs commands.

+1. I (and probably some other users) don't create a dedicated window for hierarchies. When I invoke ccls-call-hierarchy, I expect a new window for an enhanced textDocument/find* workflow. I will eagerly navigate among the items and close the window manually afterwards.

Is there an ivy-call mode? With the old implementation, when I pressed j or k in the tree window, Emacs jumped to the file:line without exiting tree navigation. With the new implementation, am I supposed to press j RET j RET j RET to navigate 3 items?

I also think the peek functionality is important; I use it a lot from ivy-xref, for example. There's a treemacs-peek command that apparently does something similar to ivy-call, but it only works for files. We would need to extend treemacs to be able to use this peek functionality from LSP. I'm not sure if Treemacs already provides a hook to let us implement treemacs-peek more easily.

This is useful in several workflows, for example, when I want to navigate different subclasses of a base class. I don't want to switch the focus from one window to another several times.

The old code expands 2 levels of nodes (:levels 2). The lsp-treemacs implementation does not seem to do so.

This also seems to be a limitation in Treemacs. I think this is less important that the other things, unless I'm missing some use case.

It is less important. Expanding 2 levels can sometimes be clumsy, especially for caller tree and callee tree.

Alexander-Miller commented 4 years ago

I'm not sure if Treemacs already provides a hook to let us implement treemacs-peek more easily.

It certainly wouldn't hurt to ask :wink: