From: João Távora Date: Sun, 6 Oct 2019 15:10:33 +0000 (+0100) Subject: Rework and correct major part of xref glue code X-Git-Tag: emacs-29.0.90~1616^2~524^2~4^2~307 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=9bb0331d04635002972b5208db6394ad931a4b86;p=emacs.git Rework and correct major part of xref glue code See comments of https://github.com/joaotavora/eglot/pull/314. Up to now, xref-backend-indentifier-completion-table was a gross hack that only worked sometimes. It relied on some fugly gymnastics to cache a response from :textDocument/documentSymbol and somehow used that information to build a completion table. But it doesn't work well. Summarily, LSP doesn't lend itself well to the xref interface of prompting for an arbitrary identifier and then go look for whichever type of references of that identifier. All the LSP :textDocument/{definition,references,implementation,...} methods expect to know the exact context of the search the user is about to perform, in the form of a document location. That conflicts with the xref "arbitrary string" requirement. Therefore, the slightly limited, but much more correct way, for Eglot to function is to override the user's preference of xref-prompt-for-identifier, temporarily setting it to nil in eglot--managed-mode (ideally, though, xref-prompt-for-identifier should be a function of the backend.) Later on, a possibly better behaved identifier completion table can be built on top of the :workspace/symbol LSP method. * eglot.el (xref-backend-identifier-at-point): Rewrite. (eglot--lsp-xrefs-for-method): New helper. (eglot--lsp-xref-helper): Use eglot--lsp-xrefs-for-method. (eglot--xref-definitions-method): Delete. (eglot--lsp-xref-refs): New variable. (xref-backend-references, xref-backend-definitions): Use eglot--lsp-xrefs-for-method. (eglot--managed-mode): Set xref-prompt-for-identifier to nil. (eglot--xref-reset-known-symbols, eglot--xref-known-symbols): Delete (xref-backend-identifier-completion-table): Nullify. (eglot-find-declaration, eglot-find-implementation) (eglot-find-typeDefinition): Use eglot--lsp-xref-helper. --- diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el index e3ead963610..51914d9e4ed 100644 --- a/lisp/progmodes/eglot.el +++ b/lisp/progmodes/eglot.el @@ -1193,6 +1193,7 @@ and just return it. PROMPT shouldn't end with a question mark." (add-hook 'post-self-insert-hook 'eglot--post-self-insert-hook nil t) (add-hook 'pre-command-hook 'eglot--pre-command-hook nil t) (eglot--setq-saving eldoc-documentation-function #'eglot-eldoc-function) + (eglot--setq-saving xref-prompt-for-identifier nil) (eglot--setq-saving flymake-diagnostic-functions '(eglot-flymake-backend t)) (add-function :around (local 'imenu-create-index-function) #'eglot-imenu) (flymake-mode 1) @@ -1717,16 +1718,6 @@ Calls REPORT-FN maybe if server publishes diagnostics in time." "EGLOT xref backend." (when (eglot--server-capable :definitionProvider) 'eglot)) -(defvar eglot--xref-known-symbols nil) - -(defun eglot--xref-reset-known-symbols (&rest _dummy) - "Reset `eglot--xref-reset-known-symbols'. -DUMMY is ignored." - (setq eglot--xref-known-symbols nil)) - -(advice-add 'xref-find-definitions :after #'eglot--xref-reset-known-symbols) -(advice-add 'xref-find-references :after #'eglot--xref-reset-known-symbols) - (defvar eglot--temp-location-buffers (make-hash-table :test #'equal) "Helper variable for `eglot--handling-xrefs'.") @@ -1771,102 +1762,64 @@ Try to visit the target file for a richer summary line." (xref-make summary (xref-make-file-location file line column)))) (cl-defmethod xref-backend-identifier-completion-table ((_backend (eql eglot))) - (when (eglot--server-capable :documentSymbolProvider) - (let ((server (eglot--current-server-or-lose)) - (text-id (eglot--TextDocumentIdentifier))) - (completion-table-with-cache - (lambda (string) - (setq eglot--xref-known-symbols - (mapcar - (eglot--lambda - ((SymbolInformation) name kind location containerName) - (propertize name - :textDocumentPositionParams - (list :textDocument text-id - :position (plist-get - (plist-get location :range) - :start)) - :locations (vector location) - :kind kind - :containerName containerName)) - (jsonrpc-request server - :textDocument/documentSymbol - `(:textDocument ,text-id)))) - (all-completions string eglot--xref-known-symbols)))))) + (eglot--error "cannot (yet) provide reliable completion table for LSP symbols")) (cl-defmethod xref-backend-identifier-at-point ((_backend (eql eglot))) - (when-let ((symatpt (symbol-at-point))) - (propertize (symbol-name symatpt) - :textDocumentPositionParams - (eglot--TextDocumentPositionParams)))) - -(defvar eglot--xref-definitions-method :textDocument/definition - "The LSP method to map xref-find-definitions call.") + ;; JT@19/10/09: This is a totally dummy identifier that isn't even + ;; passed to LSP. The reason for this particular wording is to + ;; construct a readable message "No references for LSP identifier at + ;; point.". See http://github.com/joaotavora/eglot/issues/314 + "LSP identifier at point.") + +(defvar eglot--lsp-xref-refs nil + "`xref' objects for overriding `xref-backend-references''s.") + +(cl-defun eglot--lsp-xrefs-for-method (method &key extra-params capability) + "Make `xref''s for METHOD, EXTRA-PARAMS, check CAPABILITY." + (unless (eglot--server-capable + (or capability + (intern + (format ":%sProvider" + (cadr (split-string (symbol-name method) + "/")))))) + (eglot--error "Sorry, this server doesn't do %s" method)) + (eglot--handling-xrefs + (mapcar + (eglot--lambda ((Location) uri range) + (eglot--xref-make (symbol-at-point) uri range)) + (jsonrpc-request + (eglot--current-server-or-lose) method (append + (eglot--TextDocumentPositionParams) + extra-params))))) + +(defun eglot--lsp-xref-helper (method) + "Helper for `eglot-find-declaration' & friends." + (let ((eglot--lsp-xref-refs (eglot--lsp-xrefs-for-method method))) + (xref-find-references "LSP identifier at point."))) (defun eglot-find-declaration () - "Find the declaration for the identifier at point. -See `xref-find-definitions' and `xref-prompt-for-identifier'." + "Find declaration for SYM, the identifier at point." (interactive) - (eglot--find-location 'declaration)) + (eglot--lsp-xref-helper :textDocument/declaration)) (defun eglot-find-implementation () - "Find the implementation for the identifier at point. -See `xref-find-definitions' and `xref-prompt-for-identifier'." + "Find implementation for SYM, the identifier at point." (interactive) - (eglot--find-location 'implementation)) + (eglot--lsp-xref-helper :textDocument/implementation)) (defun eglot-find-typeDefinition () - "Find the type definition for the identifier at point. -See `xref-find-definitions' and `xref-prompt-for-identifier'." + "Find type definition for SYM, the identifier at point." (interactive) - (eglot--find-location 'typeDefinition)) - -(defun eglot--find-location (kind) - (let* ((method-name (symbol-name kind)) - (method (intern (concat ":textDocument/" method-name))) - (capability (intern (concat ":" method-name "Provider")))) - (if (eglot--server-capable capability) - (let ((eglot--xref-definitions-method method)) - (call-interactively #'xref-find-definitions)) - (eglot--error "Server is not a %sProvider" method-name)))) - -(cl-defmethod xref-backend-definitions ((_backend (eql eglot)) identifier) - (let* ((rich-identifier - (car (member identifier eglot--xref-known-symbols))) - (definitions - (if rich-identifier - (get-text-property 0 :locations rich-identifier) - (jsonrpc-request (eglot--current-server-or-lose) - eglot--xref-definitions-method - (get-text-property - 0 :textDocumentPositionParams identifier)))) - (locations - (and definitions - (if (vectorp definitions) definitions (vector definitions))))) - (eglot--handling-xrefs - (mapcar (eglot--lambda ((Location) uri range) - (eglot--xref-make identifier uri range)) - locations)))) - -(cl-defmethod xref-backend-references ((_backend (eql eglot)) identifier) - (when (eglot--server-capable :referencesProvider) - (let ((params - (or (get-text-property 0 :textDocumentPositionParams identifier) - (let ((rich (car (member identifier eglot--xref-known-symbols)))) - (and rich - (get-text-property 0 :textDocumentPositionParams rich)))))) - (unless params - (eglot--error "Don' know where %s is in the workspace!" identifier)) - (eglot--handling-xrefs - (mapcar - (eglot--lambda ((Location) uri range) - (eglot--xref-make identifier uri range)) - (jsonrpc-request (eglot--current-server-or-lose) - :textDocument/references - (append - params - (list :context - (list :includeDeclaration t))))))))) + (eglot--lsp-xref-helper :textDocument/typeDefinition)) + +(cl-defmethod xref-backend-definitions ((_backend (eql eglot)) _identifier) + (eglot--lsp-xrefs-for-method :textDocument/definition)) + +(cl-defmethod xref-backend-references ((_backend (eql eglot)) _identifier) + (or + eglot--lsp-xref-refs + (eglot--lsp-xrefs-for-method + :textDocument/references :extra-params `(:context (:includeDeclaration t))))) (cl-defmethod xref-backend-apropos ((_backend (eql eglot)) pattern) (when (eglot--server-capable :workspaceSymbolProvider)