Closed sebastiencs closed 4 years ago
This implementation is different from what we have in dap-mouse.el . We should figure out which one is better. If we decide to go this way, I think that we should somehow unify the mouse handling here and in dap-mouse.el to avoid collisions.
We should also take into account: https://github.com/emacs-lsp/dap-mode/issues/348
I also suggest having this enabled by default and require "follow cursor mode" to be enabled explicitly. This suggestion is based on the overall feedback on lsp-ui-doc being too noisy and also based on the fact that vscode has it that way.
I might be doing something wrong but I am getting this:
Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
>=(524 nil)
(and hover (>= (point) (car bounds)) (<= (point) (cdr bounds)) (eq buffer (current-buffer)))
(if (and hover (>= (point) (car bounds)) (<= (point) (cdr bounds)) (eq buffer (current-buffer))) (progn (setq lsp-ui-doc--bounds bounds) (lsp-ui-doc--display (thing-at-point 'symbol t) (let ((result (let (...) (if result ...)))) (if result (progn (replace-regexp-in-string "\15" "" result)))))) (lsp-ui-doc--hide-frame))
(let ((bounds (or (lsp-ui-doc--extract-bounds hover) bounds))) (if (and hover (>= (point) (car bounds)) (<= (point) (cdr bounds)) (eq buffer (current-buffer))) (progn (setq lsp-ui-doc--bounds bounds) (lsp-ui-doc--display (thing-at-point 'symbol t) (let ((result (let ... ...))) (if result (progn (replace-regexp-in-string "\15" "" result)))))) (lsp-ui-doc--hide-frame)))
(let* ((hover input0) (contents (plist-get hover :contents)) (bounds input1) (buffer input2)) (let ((bounds (or (lsp-ui-doc--extract-bounds hover) bounds))) (if (and hover (>= (point) (car bounds)) (<= (point) (cdr bounds)) (eq buffer (current-buffer))) (progn (setq lsp-ui-doc--bounds bounds) (lsp-ui-doc--display (thing-at-point 'symbol t) (let ((result ...)) (if result (progn ...))))) (lsp-ui-doc--hide-frame))))
lsp-ui-doc--callback((:contents [(:language "java" :value "java.lang.System") "The `System` class contains several useful class f..."]) nil #<buffer AppTest.java>)
(let ((lsp-ui-doc-position 'at-point) (lsp-ui-doc--from-mouse-current t)) (lsp-ui-doc--callback hover nil (current-buffer)))
(save-excursion (goto-char lsp-ui-doc--last-event) (let ((lsp-ui-doc-position 'at-point) (lsp-ui-doc--from-mouse-current t)) (lsp-ui-doc--callback hover nil (current-buffer))))
(closure (t) (hover) (save-excursion (goto-char lsp-ui-doc--last-event) (let ((lsp-ui-doc-position 'at-point) (lsp-ui-doc--from-mouse-current t)) (lsp-ui-doc--callback hover nil (current-buffer)))))((:contents [(:language "java" :value "java.lang.System") "The `System` class contains several useful class f..."]))
apply((closure (t) (hover) (save-excursion (goto-char lsp-ui-doc--last-event) (let ((lsp-ui-doc-position 'at-point) (lsp-ui-doc--from-mouse-current t)) (lsp-ui-doc--callback hover nil (current-buffer))))) (:contents [(:language "java" :value "java.lang.System") "The `System` class contains several useful class f..."]))
(save-current-buffer (set-buffer buf) (apply callback args))
(closure ((cleanup-hooks closure (... ... ... ... ... ... ... ... ... ... ... ... ... cl-struct-lsp--log-entry-tags cl-struct-lsp-session-tags cl-struct-lsp--workspace-tags cl-struct-lsp--registered-capability-tags lsp-mode-menu cl-struct-lsp--folding-range-tags cl-struct-lsp-watch-tags cl-struct-lsp--client-tags lsp--log-lines dap-ui-menu-items t) nil (if ... ...) (remhash cancel-token lsp--cancelable-requests)) (buf . #<buffer AppTest.java>) (hooks (kill-buffer-hook . t) (after-change-functions . t)) (id . 26) (method . "textDocument/hover") (start-time 24397 14232 898274 178000) (target-workspaces #s(lsp--workspace :ewoc nil :server-capabilities ... :registered-server-capabilities ... :root "/home/yyoncho/Sources/lsp/lsp..." :client ... :host-root nil :proc #<process jdtls> :cmd-proc #<process jdtls> :buffers ... :semantic-highlighting-faces nil :semantic-highlighting-modifier-faces nil :extra-client-capabilities nil :status initialized :metadata #<hash-table equal 0/65 0x157027aafdc9> :watches #<hash-table equal 0/65 0x157029714ea5> :workspace-folders nil :last-id 0 :status-string nil :shutdown-action nil :diagnostics #<hash-table equal 0/65 0x157029714ec5> :work-done-tokens #<hash-table equal 0/65 0x157027a588e1>)) (cancel-token . :lsp-ui-doc-hover) (no-merge) (cancel-callback) (error-callback) (mode . tick) (callback closure (t) (hover) (save-excursion ... ...)) (body :jsonrpc "2.0" :method "textDocument/hover" :params (:textDocument ... :position ...) :id 26) cl-struct-lsp--log-entry-tags cl-struct-lsp-session-tags cl-struct-lsp--workspace-tags cl-struct-lsp--registered-capability-tags lsp-mode-menu cl-struct-lsp--folding-range-tags cl-struct-lsp-watch-tags cl-struct-lsp--client-tags lsp--log-lines dap-ui-menu-items t) (&rest args) (save-current-buffer (set-buffer buf) (apply callback args)))((:contents [(:language "java" :value "java.lang.System") "The `System` class contains several useful class f..."]))
funcall((closure ((cleanup-hooks closure ((buf . #<buffer AppTest.java>) (hooks ... ...) (id . 26) (method . "textDocument/hover") (start-time 24397 14232 898274 178000) (target-workspaces ...) (cancel-token . :lsp-ui-doc-hover) (no-merge) (cancel-callback) (error-callback) (mode . tick) (callback closure ... ... ...) (body :jsonrpc "2.0" :method "textDocument/hover" :params ... :id 26) cl-struct-lsp--log-entry-tags cl-struct-lsp-session-tags cl-struct-lsp--workspace-tags cl-struct-lsp--registered-capability-tags lsp-mode-menu cl-struct-lsp--folding-range-tags cl-struct-lsp-watch-tags cl-struct-lsp--client-tags lsp--log-lines dap-ui-menu-items t) nil (if (buffer-live-p buf) (progn ...)) (remhash cancel-token lsp--cancelable-requests)) (buf . #<buffer AppTest.java>) (hooks (kill-buffer-hook . t) (after-change-functions . t)) (id . 26) (method . "textDocument/hover") (start-time 24397 14232 898274 178000) (target-workspaces #s(lsp--workspace :ewoc nil :server-capabilities (:textDocumentSync ... :hoverProvider t :completionProvider ... :signatureHelpProvider ... :definitionProvider t :typeDefinitionProvider t :implementationProvider t :referencesProvider t :documentHighlightProvider t :documentSymbolProvider t :workspaceSymbolProvider t :codeLensProvider ... :documentOnTypeFormattingProvider ... :executeCommandProvider ... :workspace ... :callHierarchyProvider t :selectionRangeProvider t) :registered-server-capabilities (... ... ... ... ... ... ... ...) :root "/home/yyoncho/Sources/lsp/lsp-mode/tes..." :client #s(lsp--client :language-id nil :add-on? nil :new-connection ... :ignore-regexps nil :ignore-messages nil :notification-handlers #<hash-table equal 5/65 0x15702abcaaa9> :request-handlers #<hash-table equal 1/65 0x15702abcacd9> :response-handlers #<hash-table eql 6/65 0x15702abcb525> :prefix-function nil :uri-handlers #<hash-table equal 1/65 0x15702abcad19> :action-handlers #<hash-table equal 9/65 0x15702abcacf9> :major-modes ... :activation-fn nil :priority 0 :server-id jdtls :multi-root t :initialization-options #f(compiled-function () #<bytecode 0x15702abca98d>) :custom-capabilities nil :library-folders-fn #f(compiled-function (workspace) #<bytecode 0x15702abca99d>) :before-file-open-fn #f(compiled-function (workspace) #<bytecode 0x15702abca9d5>) :initialized-fn #f(compiled-function (workspace) #<bytecode 0x15702abcaa11>) :remote? nil :completion-in-comments? t :path->uri-fn nil :uri->path-fn nil :environment-fn nil :after-open-fn nil :async-request-handlers #<hash-table equal 0/65 0x15702abcb985> :download-server-fn lsp-java--ensure-server :download-in-progress? nil :buffers nil) :host-root nil :proc #<process jdtls> :cmd-proc #<process jdtls> :buffers (#<buffer AppTest.java>) :semantic-highlighting-faces nil :semantic-highlighting-modifier-faces nil :extra-client-capabilities nil :status initialized :metadata #<hash-table equal 0/65 0x157027aafdc9> :watches #<hash-table equal 0/65 0x157029714ea5> :workspace-folders nil :last-id 0 :status-string nil :shutdown-action nil :diagnostics #<hash-table equal 0/65 0x157029714ec5> :work-done-tokens #<hash-table equal 0/65 0x157027a588e1>)) (cancel-token . :lsp-ui-doc-hover) (no-merge) (cancel-callback) (error-callback) (mode . tick) (callback closure (t) (hover) (save-excursion (goto-char lsp-ui-doc--last-event) (let ... ...))) (body :jsonrpc "2.0" :method "textDocument/hover" :params (:textDocument (:uri "file:///home/yyoncho/Sources/lsp/lsp-m...") :position (:line 27 :character 9)) :id 26) cl-struct-lsp--log-entry-tags cl-struct-lsp-session-tags cl-struct-lsp--workspace-tags cl-struct-lsp--registered-capability-tags lsp-mode-menu cl-struct-lsp--folding-range-tags cl-struct-lsp-watch-tags cl-struct-lsp--client-tags lsp--log-lines dap-ui-menu-items t) (&rest args) (save-current-buffer (set-buffer buf) (apply callback args))) (:contents [(:language "java" :value "java.lang.System") "The `System` class contains several us..."]))
(progn (funcall callback (if no-merge results (lsp--merge-results (-map #'cl-rest results) method))))
(if (and (not (eq (length errors) (length workspaces))) (eq (+ (length errors) (length results)) (length workspaces))) (progn (funcall callback (if no-merge results (lsp--merge-results (-map #'cl-rest results) method)))))
(closure (... ... ... ... ... ... cl-struct-lsp--log-entry-tags cl-struct-lsp-session-tags cl-struct-lsp--workspace-tags cl-struct-lsp--registered-capability-tags lsp-mode-menu cl-struct-lsp--folding-range-tags cl-struct-lsp-watch-tags cl-struct-lsp--client-tags lsp--log-lines ...) (result) (let* ... ...) (if ... ...))((:contents [(:language "java" :value "java.lang.System") "The `System` class contains several useful class f..."]))
funcall((closure ((errors) (results (#s(lsp--workspace :ewoc nil :server-capabilities ... :registered-server-capabilities ... :root "/home/yyoncho/Sources/lsp/lsp-mode/tes..." :client ... :host-root nil :proc #<process jdtls> :cmd-proc #<process jdtls> :buffers ... :semantic-highlighting-faces nil :semantic-highlighting-modifier-faces nil :extra-client-capabilities nil :status initialized :metadata #<hash-table equal 0/65 0x157027aafdc9> :watches #<hash-table equal 0/65 0x157029714ea5> :workspace-folders nil :last-id 0 :status-string nil :shutdown-action nil :diagnostics #<hash-table equal 0/65 0x157029714ec5> :work-done-tokens #<hash-table equal 0/65 0x157027a588e1>) :contents [... "The `System` class contains several us..."])) (workspaces #s(lsp--workspace :ewoc nil :server-capabilities (:textDocumentSync ... :hoverProvider t :completionProvider ... :signatureHelpProvider ... :definitionProvider t :typeDefinitionProvider t :implementationProvider t :referencesProvider t :documentHighlightProvider t :documentSymbolProvider t :workspaceSymbolProvider t :codeLensProvider ... :documentOnTypeFormattingProvider ... :executeCommandProvider ... :workspace ... :callHierarchyProvider t :selectionRangeProvider t) :registered-server-capabilities (... ... ... ... ... ... ... ...) :root "/home/yyoncho/Sources/lsp/lsp-mode/tes..." :client #s(lsp--client :language-id nil :add-on? nil :new-connection ... :ignore-regexps nil :ignore-messages nil :notification-handlers #<hash-table equal 5/65 0x15702abcaaa9> :request-handlers #<hash-table equal 1/65 0x15702abcacd9> :response-handlers #<hash-table eql 6/65 0x15702abcb525> :prefix-function nil :uri-handlers #<hash-table equal 1/65 0x15702abcad19> :action-handlers #<hash-table equal 9/65 0x15702abcacf9> :major-modes ... :activation-fn nil :priority 0 :server-id jdtls :multi-root t :initialization-options #f(compiled-function () #<bytecode 0x15702abca98d>) :custom-capabilities nil :library-folders-fn #f(compiled-function (workspace) #<bytecode 0x15702abca99d>) :before-file-open-fn #f(compiled-function (workspace) #<bytecode 0x15702abca9d5>) :initialized-fn #f(compiled-function (workspace) #<bytecode 0x15702abcaa11>) :remote? nil :completion-in-comments? t :path->uri-fn nil :uri->path-fn nil :environment-fn nil :after-open-fn nil :async-request-handlers #<hash-table equal 0/65 0x15702abcb985> :download-server-fn lsp-java--ensure-server :download-in-progress? nil :buffers nil) :host-root nil :proc #<process jdtls> :cmd-proc #<process jdtls> :buffers (#<buffer AppTest.java>) :semantic-highlighting-faces nil :semantic-highlighting-modifier-faces nil :extra-client-capabilities nil :status initialized :metadata #<hash-table equal 0/65 0x157027aafdc9> :watches #<hash-table equal 0/65 0x157029714ea5> :workspace-folders nil :last-id 0 :status-string nil :shutdown-action nil :diagnostics #<hash-table equal 0/65 0x157029714ec5> :work-done-tokens #<hash-table equal 0/65 0x157027a588e1>)) (no-merge) (method . "textDocument/hover") (callback closure ((cleanup-hooks closure ... nil ... ...) (buf . #<buffer AppTest.java>) (hooks ... ...) (id . 26) (method . "textDocument/hover") (start-time 24397 14232 898274 178000) (target-workspaces ...) (cancel-token . :lsp-ui-doc-hover) (no-merge) (cancel-callback) (error-callback) (mode . tick) (callback closure ... ... ...) (body :jsonrpc "2.0" :method "textDocument/hover" :params ... :id 26) cl-struct-lsp--log-entry-tags cl-struct-lsp-session-tags cl-struct-lsp--workspace-tags cl-struct-lsp--registered-capability-tags lsp-mode-menu cl-struct-lsp--folding-range-tags cl-struct-lsp-watch-tags cl-struct-lsp--client-tags lsp--log-lines dap-ui-menu-items t) (&rest args) (save-current-buffer (set-buffer buf) (apply callback args))) cl-struct-lsp--log-entry-tags cl-struct-lsp-session-tags cl-struct-lsp--workspace-tags cl-struct-lsp--registered-capability-tags lsp-mode-menu cl-struct-lsp--folding-range-tags cl-struct-lsp-watch-tags cl-struct-lsp--client-tags lsp--log-lines dap-ui-menu-items t) (result) (let* ((v (cons lsp--cur-workspace result))) (if (eq result :error) (setq errors (cons v errors)) (setq results (cons v results)))) (if (and (not (eq ... ...)) (eq (+ ... ...) (length workspaces))) (progn (funcall callback (if no-merge results ...))))) (:contents [(:language "java" :value "java.lang.System") "The `System` class contains several us..."]))
(closure (... ... ... ... ... ... ... ... ... ... ... ... ... ... ... ...) (result) (lsp--request-cleanup-hooks id) (funcall callback result))((:contents [(:language "java" :value "java.lang.System") "The `System` class contains several useful class f..."]))
funcall((closure ((callback closure (... ... ... ... ... ... cl-struct-lsp--log-entry-tags cl-struct-lsp-session-tags cl-struct-lsp--workspace-tags cl-struct-lsp--registered-capability-tags lsp-mode-menu cl-struct-lsp--folding-range-tags cl-struct-lsp-watch-tags cl-struct-lsp--client-tags lsp--log-lines dap-ui-menu-items t) (result) (let* ... ...) (if ... ...)) (callback closure (... ... ... ... ... ... ... ... ... ... ... ... ... ... cl-struct-lsp--log-entry-tags cl-struct-lsp-session-tags cl-struct-lsp--workspace-tags cl-struct-lsp--registered-capability-tags lsp-mode-menu cl-struct-lsp--folding-range-tags cl-struct-lsp-watch-tags cl-struct-lsp--client-tags lsp--log-lines dap-ui-menu-items t) (&rest args) (save-current-buffer ... ...)) (cleanup-hooks closure (... ... ... ... ... ... ... ... ... ... ... ... ... cl-struct-lsp--log-entry-tags cl-struct-lsp-session-tags cl-struct-lsp--workspace-tags cl-struct-lsp--registered-capability-tags lsp-mode-menu cl-struct-lsp--folding-range-tags cl-struct-lsp-watch-tags cl-struct-lsp--client-tags lsp--log-lines dap-ui-menu-items t) nil (if ... ...) (remhash cancel-token lsp--cancelable-requests)) (buf . #<buffer AppTest.java>) (hooks (kill-buffer-hook . t) (after-change-functions . t)) (id . 26) (method . "textDocument/hover") (start-time 24397 14232 898274 178000) (target-workspaces #s(lsp--workspace :ewoc nil :server-capabilities ... :registered-server-capabilities ... :root "/home/yyoncho/Sources/lsp/lsp-mo..." :client ... :host-root nil :proc #<process jdtls> :cmd-proc #<process jdtls> :buffers ... :semantic-highlighting-faces nil :semantic-highlighting-modifier-faces nil :extra-client-capabilities nil :status initialized :metadata #<hash-table equal 0/65 0x157027aafdc9> :watches #<hash-table equal 0/65 0x157029714ea5> :workspace-folders nil :last-id 0 :status-string nil :shutdown-action nil :diagnostics #<hash-table equal 0/65 0x157029714ec5> :work-done-tokens #<hash-table equal 0/65 0x157027a588e1>)) (cancel-token . :lsp-ui-doc-hover) (no-merge) (cancel-callback) (error-callback) (mode . tick) (callback closure (t) (hover) (save-excursion ... ...)) (body :jsonrpc "2.0" :method "textDocument/hover" :params (:textDocument ... :position ...) :id 26) cl-struct-lsp--log-entry-tags cl-struct-lsp-session-tags cl-struct-lsp--workspace-tags cl-struct-lsp--registered-capability-tags lsp-mode-menu cl-struct-lsp--folding-range-tags cl-struct-lsp-watch-tags cl-struct-lsp--client-tags lsp--log-lines dap-ui-menu-items t) (result) (lsp--request-cleanup-hooks id) (funcall callback result)) (:contents [(:language "java" :value "java.lang.System") "The `System` class contains seve..."]))
I might be doing something wrong but I am getting this
Thanks, I assumed the server always send range of the hover. Should be fixed with a3dee37
(#493)
I think that we should somehow unify the mouse handling here and in dap-mouse.el to avoid collisions.
I agree.
What are the requirements for dap-mouse ? I'm willing to help integrating this implementation in dap-mouse.
We should also take into account: emacs-lsp/dap-mode#348
Good point, I will take a look
What are the requirements for dap-mouse ?
It shows hover information for the variable at the point. I think that the collision point is that it should "override" lsp-ui hover when there is an active debug session. I think that this is what makes sense for that case, not sure if there is an alternative. Is this the info you are looking for?
And one more thing that I would like to have - standardize the sizes of the popups. So, the signature info(see https://github.com/emacs-lsp/lsp-mode/pull/1999), ui doc, dap-mode hovers, etc. should have the same width and eventually the same max height by default.
It shows hover information for the variable at the point. I think that the collision point is that it should "override" lsp-ui hover when there is an active debug session. I think that this is what makes sense for that case, not sure if there is an alternative. Is this the info you are looking for?
Yes ok, make sense.
And one more thing that I would like to have - standardize the sizes of the popups.
hmm I'm not sure if it's a good idea, there are symbols where the documentation is just a short word, such as usize
or ()
in rust.
Having a big/medium frame width for such documentation would be a waste of space IMO, and the frame would hide the rest of the buffer, just for empty space.
Setting the same max width and height between those projects would make more sense I think ?
hmm I'm not sure if it's a good idea, there are symbols where the documentation is just a short word, such as
usize
or()
in rust. Having a big/medium frame width for such documentation would be a waste of space IMO, and the frame would hide the rest of the buffer, just for empty space.
Agreed - I tested the vscode equivalent - when it is single liner it shrinks the width. Let's have only this as an exception.
and the frame would hide the rest of the buffer, just for empty space.
Just want to clarify that I want a fixed minimum size only for width, not for height. So in general, if we decide to enforce minimal width it will hide only one line.
ATM this is the default behavior: https://preview.redd.it/5tnafm1fk7k51.png?width=1948&format=png&auto=webp&s=818e135578a1d8dc27f748de40cc9dd67bd81e2e and I think this is not fine.
I also suggest having this enabled by default and require "follow cursor mode" to be enabled explicitly. This suggestion is based on the overall feedback on lsp-ui-doc being too noisy and also based on the fact that vscode has it that way.
Following vscode is the right thing to do. But there is a difference to consider: vscode users rely heavily on the mouse for pretty much everything, emacs users don't use it (I think ?). At least, in my case, I never ever use the mouse on emacs, I wouldn't try to move my mouse around to see if some frame show up.
What about having the default with the doc showing with the mouse AND with the cursor at the top/bottom right of the frame ? We can set a smaller default height, but at least let the user know that there is a doc available. I worry that if we don't show the frame (whatever its size) by default with the cursor, the user will never know this feature exist.
ATM this is the default behavior: preview.redd.it/5tnafm1fk7k51.png?width=1948&format=png&auto=webp&s=818e135578a1d8dc27f748de40cc9dd67bd81e2e and I think this is not fine.
I agree, the size is not adapted.
The maximum height/width were introduced before the at-point
position, if I remember correctly.
Should we make 2 other variables: lsp-ui-doc-max-{width,height}-at-point
?
A max-height
for a frame in a corner might be different than what the user want for the frame at point
I also suggest having this enabled by default and require "follow cursor mode" to be enabled explicitly. This suggestion is based on the overall feedback on lsp-ui-doc being too noisy and also based on the fact that vscode has it that way.
Following vscode is the right thing to do. But there is a difference to consider: vscode users rely heavily on the mouse for pretty much everything, emacs users don't use it (I think ?). At least, in my case, I never ever use the mouse on emacs, I wouldn't try to move my mouse around to see if some frame show up.
This is a fair point. The whole suggestion with making mouse follow mode the default one is to make lsp-ui mode less intrusive by default.
What about having the default with the doc showing with the mouse AND with the cursor at the top/bottom right of the frame ? We can set a smaller default height, but at least let the user know that there is a doc available.
This will definitely help. IMO we should have smaller defaults no matter what we decide for the rest of the points.
that if we don't show the frame (whatever its size) by default with the cursor, the user will never know this feature exist.
Yes. It always has been about the fact that not enabling feature by default makes the feature undiscoverable. IMO "follow mouse" will be
@yyoncho The PR is ready to be merged. Let's change the default size in another PR ?
We should also take into account: emacs-lsp/dap-mode#348
This is handled in the PR, it won't make issue with prefix keys
Is there an option to enable only mouse hover?
I added the defcustom lsp-ui-doc-show-with-cursor
And changed some defaults:
lsp-ui-doc-position
to top
lsp-ui-doc-max-height
to 15I also made some changes with how the frame is focused and scrolled with the mouse. It should be more easy the focus/unfocus it now. Since focusing a frame can depends on the windows manager, it would be great to have some more tests
The PR is not ready to be merged yet. It adds showing the doc at mouse position. If someone is willing to test