Don't use `lsp-execute-command' (emacs-lsp#289)

* Don't use `lsp-execute-command' `lsp-execute-command' is a liability that needs to be removed; use lsp's :action-handlers for the reference and implementation lenses instead. Also remove the duplicate definition of the reference and implementation lens action functions. * `lsp-show-xrefs': supply DISPLAY argument * Fix CI byte-compile Declare `lsp-lens-backend' where necessary.
renatofdds · Nov 5, 2020 · f43b00a · f43b00a
1 parent cfe6ae0
commit f43b00a
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 44 deletions.
diff --git a/lsp-java-boot.el b/lsp-java-boot.el
@@ -75,6 +75,8 @@
 Store CALLBACK to use it `sts/highlight'."
   (setq-local lsp-java-boot--callback callback))
 
+(defvar lsp-lens-backends)
+(declare-function lsp-lens-refresh "lsp-lens" (buffer-modified? &optional buffer))
 ;;;###autoload
 (define-minor-mode lsp-java-boot-lens-mode
   "Toggle code-lens overlays."
@@ -84,6 +86,7 @@ Store CALLBACK to use it `sts/highlight'."
   :lighter "BLens"
   (cond
    (lsp-java-boot-lens-mode
+    (require 'lsp-lens)
     (setq-local lsp-lens-backends (cl-pushnew 'lsp-java-boot--lens-backend lsp-lens-backends))
     (lsp-lens-refresh t))
    (t (setq-local lsp-lens-backends (delete 'lsp-java-boot--lens-backend lsp-lens-backends))

diff --git a/lsp-java.el b/lsp-java.el
@@ -626,16 +626,16 @@ FULL specify whether full or incremental build will be performed."
   (unless (file-directory-p path)
     (make-directory path t)))
 
-(cl-defmethod lsp-execute-command
-  (_server (_command (eql java.show.references)) params)
+(lsp-defun lsp-java--show-references ((&Command :arguments? params))
+  ;; (_server (_command (eql java.show.references)) params)
   (if-let (refs (seq-elt params 2))
-      (xref--show-xrefs (lsp--locations-to-xref-items refs) nil)
+      (lsp-show-xrefs (lsp--locations-to-xref-items refs) nil t)
     (user-error "No references")))
 
-(cl-defmethod lsp-execute-command
-  (_server (_command (eql java.show.implementations)) params)
+(lsp-defun lsp-java--show-implementations ((&Command :arguments? params))
+  ;; (_server (_command (eql java.show.implementations)) params)
   (if-let (refs (seq-elt params 2))
-      (xref--show-xrefs (lsp--locations-to-xref-items refs) nil)
+      (lsp-show-xrefs (lsp--locations-to-xref-items refs) nil nil)
     (user-error "No implementations")))
 
 (defun lsp-java--get-java-version ()
@@ -977,18 +977,6 @@ current symbol."
   "Return the workspace corresponding FILE-URI."
   (lsp-find-workspace 'jdtls (lsp--uri-to-path file-uri)))
 
-(cl-defmethod lsp-execute-command
-  (_server (_command (eql java.show.references)) params)
-  (if-let (refs (cl-third (append params nil)))
-      (lsp-show-xrefs (lsp--locations-to-xref-items refs) nil t)
-    (user-error "No references")))
-
-(cl-defmethod lsp-execute-command
-  (_server (_command (eql java.show.implementations)) params)
-  (if-let (refs (cl-third (append params nil)))
-      (lsp-show-xrefs (lsp--locations-to-xref-items refs) nil t)
-    (user-error "No implementations")))
-
 (add-to-list 'global-mode-string (list '(t lsp-java-progress-string)))
 
 (defun lsp-java-boot--workspace-execute-client-command (_jdt-ls-workspace params)
@@ -1392,7 +1380,9 @@ current symbol."
                        ("java.action.generateAccessorsPrompt" #'lsp-java--generate-accessors-prompt)
                        ("java.action.generateConstructorsPrompt" #'lsp-java--generate-constructors-prompt)
                        ("java.action.applyRefactoringCommand" #'lsp-java--apply-refactoring-command)
-                       ("java.action.rename" 'lsp-java--action-rename))
+                       ("java.action.rename" #'lsp-java--action-rename)
+                       ("java.show.references" #'lsp-java--show-references)
+                       ("java.show.implementations" #'lsp-java--show-implementations))
   :uri-handlers (ht ("jdt" #'lsp-java--resolve-uri))
   :initialization-options (lambda ()
                             (list :settings (lsp-configuration-section "java")
@@ -1499,6 +1489,8 @@ current symbol."
 
 ;; lsp-java run
 
+(defvar lsp-lens-backends)
+(declare-function lsp-lens-refresh "lsp-lens" (buffer-modified? &optional buffer))
 ;;;###autoload
 (define-minor-mode lsp-java-lens-mode
   "Toggle run/debug overlays."
@@ -1508,6 +1500,7 @@ current symbol."
   :lighter nil
   (cond
    (lsp-java-lens-mode
+    (require 'lsp-lens)
     (setq-local lsp-lens-backends (cl-pushnew #'lsp-java-lens-backend lsp-lens-backends))
     (lsp-lens-refresh t))
    (t (setq-local lsp-lens-backends (delete #'lsp-java-lens-backend lsp-lens-backends)))))

diff --git a/lsp-jt.el b/lsp-jt.el
@@ -32,6 +32,9 @@
 (require 'lsp-java)
 (require 'treemacs)
 
+(eval-when-compile
+  (require 'lsp-treemacs))
+
 (declare-function dap-debug "ext:dap-mode")
 
 (defvar lsp-jt--refresh-timer nil)
@@ -248,6 +251,8 @@
        (funcall callback all-lenses lsp--cur-version)))
    :mode 'detached))
 
+(defvar lsp-lens-backends)
+(declare-function lsp-lens-refresh "lsp-lens" (buffer-modified? &optional buffer))
 ;;;###autoload
 (define-minor-mode lsp-jt-lens-mode
   "Toggle code-lens overlays."
@@ -258,6 +263,7 @@
   (let ((buffer (current-buffer)))
     (cond
      (lsp-jt-lens-mode
+      (require 'lsp-lens)
       (setq-local lsp-lens-backends (cl-pushnew 'lsp-jt-lens-backend lsp-lens-backends))
       (lsp-lens-refresh t)
       (add-hook 'lsp-jt-status-updated-hook
@@ -313,20 +319,19 @@
     (treemacs-create-icon :file "test-error.png" :extensions (java-test-error) :fallback "-")
     (treemacs-create-icon :file "history.png" :extensions (java-test-pending) :fallback "-")))
 
+(lsp-treemacs-define-action lsp-jt-run (:data)
+  "Run test from browser."
+  (lsp-jt--start-test data t))
+
 (defvar lsp-jt-mode-map
   (-doto (make-sparse-keymap)
     (define-key (kbd "x") #'lsp-jt-run)
     (define-key (kbd "d") #'lsp-jt-debug)
     (define-key (kbd "R") #'lsp-jt-browser-refresh))
   "Keymap for `lsp-jt-mode'.")
-
 (define-minor-mode lsp-jt-mode "Java Test Mode"
   nil nil lsp-jt-mode-map)
 
-(lsp-treemacs-define-action lsp-jt-run (:data)
-  "Run test from browser."
-  (lsp-jt--start-test data t))
-
 (lsp-treemacs-define-action lsp-jt-debug (:data)
   "Debug from browser."
   (lsp-jt--start-test data nil))
@@ -393,35 +398,42 @@
 (lsp-defun lsp-jt--render-test-node ((test-item &as &jt:TestItem :display-name :level :id
                                                 :location (loc &as &Location :uri) :full-name))
   `(:key ,id
-         :label ,display-name
-         :icon ,(lsp-jt--get-test-icon id level)
-         ,@(unless (eq level 4)
-             (list :children-async (lambda (_ callback)
-                                     (lsp-jt-search
-                                      uri
-                                      level
-                                      full-name
-                                      (lambda (items)
-                                        (funcall callback
-                                                 (-map
-                                                  #'lsp-jt--render-test-node
-                                                  items)))))))
-         :ret-action ,(lambda ()
-                        (interactive)
-                        (lsp-goto-location loc))
-         :actions (["Run Test"   lsp-jt-run]
-                   ["Debug Test" lsp-jt-debug]
-                   ["Refresh"    lsp-jt-browser-refresh])
-         :data ,test-item))
+    :label ,display-name
+    :icon ,(lsp-jt--get-test-icon id level)
+    ,@(unless (eq level 4)
+        (list :children-async (lambda (_ callback)
+                                (lsp-jt-search
+                                 uri
+                                 level
+                                 full-name
+                                 (lambda (items)
+                                   (funcall callback
+                                            (-map
+                                             #'lsp-jt--render-test-node
+                                             items)))))))
+    :ret-action ,(lambda ()
+                   (interactive)
+                   (lsp-goto-location loc))
+    :actions (["Run Test"   lsp-jt-run]
+              ["Debug Test" lsp-jt-debug]
+              ["Refresh"    lsp-jt-browser-refresh])
+    :data ,test-item))
+
+(declare-function lsp-treemacs-generic-refresh "lsp-treemacs" ())
+(declare-function
+ lsp-treemacs-render "lsp-treemacs"
+ (tree title expand-depth &optional buffer-name right-click-actions))
 
 (defun lsp-jt-browser-refresh ()
   (interactive)
   (with-current-buffer "*Java Tests*"
+    (require 'lsp-treemacs)
     (lsp-treemacs-generic-refresh)))
 
 ;;;###autoload
 (defun lsp-jt-browser ()
   (interactive)
+  (require 'lsp-treemacs)
   (select-window
    (display-buffer-in-side-window
     (lsp-treemacs-render