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.76k stars 875 forks source link

lsp-clojure--show-references doesn't work with consult #3711

Closed markgdawson closed 2 years ago

markgdawson commented 2 years ago

Thank you for the bug report

Bug description

When consult is installed, calls to lsp-clojure--show-references throw an error.

I believe the second argument to the call to lsp-show-xrefs in lsp-clojure--show-references should be nil rather than t.

This argument ends up being passed to the function references by the variable xref-show-xrefs-function. By default this is set to xref--show-xref-buffer, however the documentation of this variable does suggest that t isn't a supported value:

DISPLAY-ACTION indicates where the target location should be displayed. The possible values are nil, ‘window’ meaning the other window, or ‘frame’ meaning the other frame.

A value of t does however "work" with the default emacs function/implementation (i.e. xref--show-xref-buffer), despite what the docs suggest.

When consult is used, it provides an alternative implementation of xref (seen here) which does not support the undocumented t as a value, and therefore throws an error.

Steps to reproduce

Install consult and click on an lsp lens. Emacs will throw an error "No clause matching t". Adding that clause into consult-xref fixes the issue.

Expected behavior

View the consult xref display in the minibuffer.

Which Language Server did you use?

Clojure

OS

Linux

Error callstack

Debugger entered--Lisp error: (error "No clause matching ‘t’")
  error("No clause matching `%S'" t)
  (let ((x469 display)) (error "No clause matching `%S'" x469))
  (cond ((eq display 'frame) 'nil) ((eq display 'window) (let nil #'switch-to-buffer-other-window)) ((null display) (let nil #'switch-to-buffer)) (t (let ((x469 display)) (error "No clause matching `%S'" x469))))
  (and t (cond ((eq display 'frame) 'nil) ((eq display 'window) (let nil #'switch-to-buffer-other-window)) ((null display) (let nil #'switch-to-buffer)) (t (let ((x469 display)) (error "No clause matching `%S'" x469)))))
  (let* ((fun (and t (cond ((eq display 'frame) 'nil) ((eq display 'window) (let nil #'switch-to-buffer-other-window)) ((null display) (let nil #'switch-to-buffer)) (t (let (...) (error "No clause matching `%S'" x469))))))) (if fun (consult-xref--preview fun) nil))
  (list :prompt "Go to xref: " :history 'consult-xref--history :require-match t :sort nil :category 'consult-xref :group #'consult-xref--group :state (let* ((fun (and t (cond ((eq display ...) 'nil) ((eq display ...) (let nil ...)) ((null display) (let nil ...)) (t (let ... ...)))))) (if fun (consult-xref--preview fun) nil)) :lookup (apply-partially #'consult--lookup-prop 'consult-xref))
  (append (consult--customize-get #'consult-xref) (list :prompt "Go to xref: " :history 'consult-xref--history :require-match t :sort nil :category 'consult-xref :group #'consult-xref--group :state (let* ((fun (and t (cond (... ...) (... ...) (... ...) (t ...))))) (if fun (consult-xref--preview fun) nil)) :lookup (apply-partially #'consult--lookup-prop 'consult-xref)))
  (apply #'consult--read candidates (append (consult--customize-get #'consult-xref) (list :prompt "Go to xref: " :history 'consult-xref--history :require-match t :sort nil :category 'consult-xref :group #'consult-xref--group :state (let* ((fun (and t (cond ... ... ... ...)))) (if fun (consult-xref--preview fun) nil)) :lookup (apply-partially #'consult--lookup-prop 'consult-xref))))
  (if (cdr candidates) (apply #'consult--read candidates (append (consult--customize-get #'consult-xref) (list :prompt "Go to xref: " :history 'consult-xref--history :require-match t :sort nil :category 'consult-xref :group #'consult-xref--group :state (let* ((fun (and t ...))) (if fun (consult-xref--preview fun) nil)) :lookup (apply-partially #'consult--lookup-prop 'consult-xref)))) (get-text-property 0 'consult-xref (car candidates)))
  (xref-pop-to-location (if (cdr candidates) (apply #'consult--read candidates (append (consult--customize-get #'consult-xref) (list :prompt "Go to xref: " :history 'consult-xref--history :require-match t :sort nil :category 'consult-xref :group #'consult-xref--group :state (let* ((fun ...)) (if fun (consult-xref--preview fun) nil)) :lookup (apply-partially #'consult--lookup-prop 'consult-xref)))) (get-text-property 0 'consult-xref (car candidates))) display)
  (let* ((consult-xref--fetcher fetcher) (candidates (consult-xref--candidates)) (display (alist-get 'display-action alist))) (xref-pop-to-location (if (cdr candidates) (apply #'consult--read candidates (append (consult--customize-get #'consult-xref) (list :prompt "Go to xref: " :history 'consult-xref--history :require-match t :sort nil :category 'consult-xref :group #'consult-xref--group :state (let* (...) (if fun ... nil)) :lookup (apply-partially #'consult--lookup-prop 'consult-xref)))) (get-text-property 0 'consult-xref (car candidates))) display))
  consult-xref(#f(compiled-function (&rest _) #<bytecode 0xb7e8468f4f0bd24>) ((window . #<window 685 on core.clj>) (display-action . t)))
  lsp-show-xrefs(<removed xref data> t t)
  (let* ((args (plist-get input0 :arguments))) (lsp-show-xrefs (lsp--locations-to-xref-items (lsp-request "textDocument/references" (lsp--make-reference-params (lsp--text-document-position-params (list :uri (seq-elt args 0)) (list :line (1- ...) :character (1- ...)))))) t t))
  lsp-clojure--show-references((:title "3 references" :command "code-lens-references" :arguments ["file:///filename..." 57 7]))
  lsp--execute-command((:title "3 references" :command "code-lens-references" :arguments ["file:///filename..." 57 7]))
  #f(compiled-function () (interactive nil) #<bytecode 0x15eb5b1f13c8ac3e>)()
  funcall-interactively(#f(compiled-function () (interactive nil) #<bytecode 0x15eb5b1f13c8ac3e>))
  command-execute(#f(compiled-function () (interactive nil) #<bytecode 0x15eb5b1f13c8ac3e>))

Anything else?

No response

ericdallo commented 2 years ago

Makes sense, fixed on master, thank you!