emacs-lsp / lsp-mode

Emacs client/library for the Language Server Protocol
https://emacs-lsp.github.io/lsp-mode
GNU General Public License v3.0
4.82k stars 895 forks source link

[Feature] go to next/previous occurence of symbol in buffer #821

Open thanhvg opened 5 years ago

thanhvg commented 5 years ago

The feature is similar spacemacs * but in spacemacs * is a generic approach it grabs not exactly the symbol but it is more like a dumb string search.

If this feature is not worth implementing, please give some hints so i can make one for myself

Thanks

yyoncho commented 5 years ago

We may implement this method by using either textDocument/documentHighlight which are scoped to the current buffer or by textDocument/references (which is not scoped to the current buffer). If you want to give it a try you may take a look at lsp--make-document-highlight-callback which is handling the rendering of the highlights. It will be good if we reuse the data for two subsequent calls to next occurrence if there are no changes.

thanhvg commented 5 years ago

Here is my attempt: call textDocument/documentHighlight parse the result to a point list and use goto-char to go.

I didn't try to reuse any data with lsp--make-document-highlight-callback because:

;;;  -*- lexical-binding: t; -*-

(defun lsp-goto-symbol-occurence (&optional backward)
  (unless (lsp--capability "documentHighlightProvider")
    (signal 'lsp-capability-not-supported (list "documentHighlightProvider")))
  (lsp--goto-symbol-occurent (point) backward))

(defun lsp--goto-symbol-occurent (a-point backward)
  (lsp-request-async "textDocument/documentHighlight"
                     (lsp--text-document-position-params)
                     (lsp--make-goto-symbol-occurence-callback a-point backward)))

(defun lsp--make-goto-symbol-occurence-callback (a-point backward)
  (lambda (highlights)
    (when (> (length highlights) 1)
      ;; map highlights to a point list
      (let* ((my--points (-map (lambda (it)
                                 (lsp--position-to-point (gethash "start" (gethash "range" it nil)))) highlights))
             (points (-sort '< my--points))
             (len (length my--points))
             (goto--index (if backward (-find-last-index (lambda (it) (< it a-point)) points)
                            (-find-index (lambda (it) (> it a-point)) points)))
             (goto-index (if goto--index goto--index (if backward (- len 1) 0)))
             (goto-point (nth goto-index points)))
        (message "occurence %S/%S"  (+ goto-index 1) len)
        (goto-char goto-point)))))

(defun lsp-goto-symbol-occurence-forward ()
  (interactive)
  (lsp-goto-symbol-occurence))

(defun lsp-goto-symbol-occurence-backward ()
  (interactive)
  (lsp-goto-symbol-occurence t))
yyoncho commented 5 years ago

Looks good!

I didn't try to reuse any data with lsp--make-document-highlight-callback because:

What I meant is:

  1. When you do next/previous and the highlights are on there is no need to get the data once more, you could use the data you already have to call the existing callback and get the highlights for free.
  2. When highlights callbacks have already downloaded the data you may store it in buffer-local and use it directly when someone does next/previous(the data will be available). Storing the data in buffer-local will not slow down highlights process but it will speed up previous/next.

IMO it would be better to pass the number of times you have to do next/previous.

E. g.

C-3 M-x lsp-goto-symbol-occurence-forward

will move 3 symbols forward. This will also simplify lsp--make-goto-symbol-occurence-callback, what do you think?

thanhvg commented 5 years ago

IMO it would be better to pass the number of times you have to do next/previous.

Great idea and thanks for your great insight.

What I want is the same feature of Spacemacs * so it never crossed my mind that it should take a prefix argument. Also it would be nice to have a go to first and go to last occurrence I think. However I fail to see prefix argument could simplify lsp--make-goto-symbol-occurence-callback as my elisp skill is limited.

Anyway that was as far as I can go, if anyone can improve on it feel free to do so.

sebastiansturm commented 5 years ago

I like the idea of being able to navigate both local symbols and workspace-wide references interactively, though it seems to me there is substantial overlap with lsp-ui-find-next-reference and lsp-ui-find-prev-reference. Not sure what would be the best way to go here: on the one hand, lsp-ui-find-{prev,next}-reference don't seem to come with the eye candy that other lsp-ui functions provide so one might argue in favor of moving them into lsp-mode proper.

On the other hand, showing the list of accessible references in an auxiliary buffer could certainly be helpful here, and would be essential if we wanted to, e.g., implement support for interactive textDocument/typeHierarchy navigation (similar to what emacs-ccls does for ccls's type hierarchy mechanism).

As far as I can see, there's already at least two polished UI implementations available that might fit the bill (lsp-treemacs and lsp-ui's imenu renderer), though both obviously require at least one extra package beyond lsp-mode itself. Based on the responses to #542, that doesn't seem to be an issue for the typical lsp-mode user, so I'd propose to (1.) move this issue/feature over to lsp-ui where a similar feature already exists (though as far as I can see the current implementation always uses project-wide references), (2.) decide which (if any) hierarchical outline renderer we want to standardize on and (3.) use (2.) to provide visual feedback. What are your opinions on this?

yyoncho commented 5 years ago

Not sure what would be the best way to go here: on the one hand, lsp-ui-find-{prev,next}-reference don't seem to come with the eye candy that other lsp-ui functions provide so one might argue in favor of moving them into lsp-mode proper.

I think that lsp-mode should host a particular functionality unless there is a valid reason not to do so(e. g. unwanted dependency like helm/ivy/etc). FTR I wasn't even aware of that feature and I vote for moving it to lsp-mode.

On the other hand, showing the list of accessible references in an auxiliary buffer could certainly be helpful here, and would be essential if we wanted to, e.g., implement support for interactive textDocument/typeHierarchy navigation (similar to what emacs-ccls does for ccls's type hierarchy mechanism).

Sounds like a good idea, we could also provide hydras.

decide which (if any) hierarchical outline renderer we want to standardize

IMO we may do what xref does - have a plain buffer ui implementation in lsp-mode(or even no UI) and allow pluging fancy GUIs(lsp-ui/lsp-treemacs). I encourage you to try what xref does when you display the references in vanilla emacs - it has similar functionality (n for next, p for previous, etc). WDYT?

sebastiansturm commented 5 years ago

sounds good to me, I'll have a go at that

dschrempf commented 1 year ago

Hi! Was there any consense as to how we should proceed? Should we use lsp-ui-find-next-reference?