Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lsp-ui-doc--extract oddity for "MarkedString[]" #204

Closed
alanz opened this issue Dec 12, 2018 · 11 comments
Closed

lsp-ui-doc--extract oddity for "MarkedString[]" #204

alanz opened this issue Dec 12, 2018 · 11 comments

Comments

@alanz
Copy link
Contributor

alanz commented Dec 12, 2018

lsp-ui-doc--extract, used to render the pop-up window for hover text, has the following fragment for processing the MarkedString[] case

   ((sequencep contents) ;; MarkedString[]
    (mapconcat 'lsp-ui-doc--extract-marked-string
               (lsp-ui-doc--filter-marked-string contents)
               "\n\n"
               ;; (propertize "\n\n" 'face '(:height 0.4))
               ))

This calls lsp-ui-doc--filter-marked-string on contents. It it not clear to me what lsp-ui-doc--filter-marked-string is intended to do, but it does invoke an idle timer function. The nett effect for me is that the hover window only sometimes shows up.

Changing this to

   ((sequencep contents) ;; MarkedString[]
    (mapconcat 'lsp-ui-doc--extract-marked-string
               contents
               "\n\n"
               ;; (propertize "\n\n" 'face '(:height 0.4))
               ))

Results in reliable display of the hover (except the very first time for a file, which I assume is an initialization issue, separate from this, see #178 )

So, is there any reason to call lsp-ui-doc--filter-marked-string ? If so, should it be a different version, without the idle timer randomly inserted in the middle?

@sebastiencs
Copy link
Member

sebastiencs commented Dec 12, 2018

but it does invoke an idle timer function

What idle timer ? I don't see an idle timer in the function

EDIT: I didn't have the last version, I will investigate why there is a timer here

lsp-ui-doc--filter-marked-string is used to not display the same text that lsp-ui-sideline is already displaying

@alanz
Copy link
Contributor Author

alanz commented Dec 12, 2018

Here is the idle timer in question: https://github.com/emacs-lsp/lsp-ui/blob/master/lsp-ui-doc.el#L256

(defun lsp-ui-doc--filter-marked-string (list-marked-string)
  (let ((groups (--separate (and (hash-table-p it)
                                 (lsp-get-renderer (gethash "language" it)))
                            (append list-marked-string nil))))
    (when-let ((marked-string (caar groups)))
      ;; Without run-with-idle-timer, echo area will be cleared after displaying the message instantly.
      (run-with-idle-timer 0 nil (lambda () (eldoc-message (lsp-ui-doc--extract-marked-string marked-string)))))
    (if lsp-ui-doc-include-signature
        list-marked-string
      (cadr groups))))

@alanz
Copy link
Contributor Author

alanz commented Dec 12, 2018

Just a note: hover (confusingly) gets used in two different ways in lsp-ui. The first is to populate the sideline with the types of all the nearby variables (a great feature).

The second is to provide a pop-up documentation window.

The problem I am seeing relates to this second usage, where often the pop-up window simply does not appear, because it is filtered out.

@sebastiencs
Copy link
Member

The problem I am seeing relates to this second usage, where often the pop-up window simply does not appear, because it is filtered out.

Do you mean the hover content is neither display in lsp-ui-doc nor lsp-ui-sideline ?

@alanz
Copy link
Contributor Author

alanz commented Dec 12, 2018

Using lsp.el and lsp-ui from melpa, I am seeing it show up on both the sideline and the hover window.

But the hover window is flaky, it will display in random positions (supposed to be bottom), will vary from bottom right corner, to middle-ish of the screen. And a lot of the time the bounding size is wrong, so the text does not fit.

And occasionally it is displayed, but will not then undisplay, even as I move around the file to different points.

@sebastiencs
Copy link
Member

That's a bug. The frame should stay at the bottom of your main frame.
Could you send screenshots or video of those behaviors ?
Just to be sure that I understand clearly

@MaskRay
Copy link
Member

MaskRay commented Dec 14, 2018

(run-with-idle-timer 0 nil (lambda () (eldoc-message (lsp-ui-doc--extract-marked-string marked-string)) is to work around a weird issue as its comment says. I don't know a better way to fix it.

What I am also unclear is whether we can remove the blinking: eldoc message disappears when the cursor moves. If the message in the echo area sticks, it will look better.

@alanz
Copy link
Contributor Author

alanz commented Dec 14, 2018

@MaskRay I have no issue with that timer, for the use-case given in the comment.

But the problem I have is that this same filter function is used when preparing to render the non-sideline hover, when it is in a pop-up frame, rather than in the message area.

@yyoncho
Copy link
Member

yyoncho commented Dec 16, 2018

I will probably fix this issue as part of the restructuring in emacs-lsp/lsp-mode#214 . As a side effect we will get the following effect: eldoc will display the method signature while the lsp-ui doc will display the hover info(e. g. you will be able to see parameter info in the UI Frame and function signature in the echo area).

@alanz
Copy link
Contributor Author

alanz commented Dec 16, 2018

If you are doing a rewrite, take cognizance of this thread on the spec: microsoft/language-server-protocol#518

Which has to do with separators when there is a list of documentation

@yyoncho
Copy link
Member

yyoncho commented Dec 16, 2018

Thanks for you pointer. We need first to unify the markup rendering in lsp-ui and lsp-mode and then do this enhancement. We also need a UI component which will do "pretty" representation of the method signatures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants