Skip to content

Use posframe library for lsp-ui-doc #459

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 25, 2020
Merged

Use posframe library for lsp-ui-doc #459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 94 additions & 95 deletions lsp-ui-doc.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
(require 'goto-addr)
(require 'markdown-mode)
(require 'cl-lib)
(require 'posframe)

(when (featurep 'xwidget-internal)
(require 'xwidget))
Expand Down Expand Up @@ -144,29 +145,14 @@ Only the `background' is used in this face."
:group 'lsp-ui-doc)

(defvar lsp-ui-doc-frame-parameters
'((left . -1)
(no-focus-on-map . t)
(min-width . 0)
(width . 0)
(min-height . 0)
(height . 0)
(internal-border-width . 1)
'((no-focus-on-map . t)
(vertical-scroll-bars . nil)
(horizontal-scroll-bars . nil)
(right-fringe . 0)
(menu-bar-lines . 0)
(tool-bar-lines . 0)
(line-spacing . 0)
(unsplittable . t)
(undecorated . t)
(top . -1)
(visibility . nil)
(mouse-wheel-frame . nil)
(no-other-frame . t)
(no-accept-focus . nil)
Copy link
Member

@kurnevsky kurnevsky Jun 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was it added explicitly?

@alanz you can set it to t to fix focus captures.

Added: though it might not work...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In our original child frame implementation, we removed the no-accept-focus so we can focus into child frame to scroll the document. There is little no issue with that approach though, i.e: mouse is not suddenly set focus to other frame.
However, posframe add no-accept-focus to t explicitly, so we need to add back no-accept-focus . nil here

(inhibit-double-buffering . t)
(drag-internal-border . t)
(no-special-glyphs . t)
(desktop-dont-save . t))
(cursor-type . box)
(drag-internal-border . t))
"Frame parameters used to create the frame.")

(defvar lsp-ui-doc-render-function nil
Expand Down Expand Up @@ -365,10 +351,7 @@ We don't extract the string that `lps-line' is already displaying."
(when (overlayp lsp-ui-doc--inline-ov)
(delete-overlay lsp-ui-doc--inline-ov))
(when (lsp-ui-doc--get-frame)
(unless lsp-ui-doc-use-webkit
(lsp-ui-doc--with-buffer
(erase-buffer)))
(make-frame-invisible (lsp-ui-doc--get-frame))))
(posframe-hide (lsp-ui-doc--make-buffer-name))))

(defun lsp-ui-doc--buffer-width ()
"Calcul the max width of the buffer."
Expand Down Expand Up @@ -403,19 +386,6 @@ We don't extract the string that `lps-line' is already displaying."
(xwidget-resize (lsp-ui-doc--webkit-get-xwidget) offset-width offset-height))
(lsp-ui-doc--move-frame (lsp-ui-doc--get-frame)))

(defun lsp-ui-doc--resize-buffer ()
"If the buffer's width is larger than the current frame, resize it."
(if lsp-ui-doc-use-webkit
(lsp-ui-doc--webkit-execute-script
"[document.querySelector('#lsp-ui-webkit').offsetWidth, document.querySelector('#lsp-ui-webkit').offsetHeight];"
'lsp-ui-doc--webkit-resize-callback)

(let* ((frame-width (frame-width))
(fill-column (min lsp-ui-doc-max-width (- frame-width 5))))
(when (> (lsp-ui-doc--buffer-width) (min lsp-ui-doc-max-width frame-width))
(lsp-ui-doc--with-buffer
(fill-region (point-min) (point-max)))))))

(defun lsp-ui-doc--mv-at-point (frame width height start-x start-y)
"Move FRAME to be where the point is.
WIDTH is the child frame width.
Expand Down Expand Up @@ -459,12 +429,11 @@ FRAME just below the symbol at point."
(if (eq lsp-ui-doc-position 'at-point)
(lsp-ui-doc--mv-at-point frame width height left top)
(set-frame-position frame
(max (- frame-right width 10 (frame-char-width)) 10)
(max (- frame-right width ) 0)
(pcase lsp-ui-doc-position
('top (+ top 10))
('top top)
('bottom (- (lsp-ui-doc--line-height 'mode-line)
height
10)))))))
height)))))))

(defun lsp-ui-doc--visit-file (filename)
"Visit FILENAME in the parent frame."
Expand Down Expand Up @@ -495,26 +464,31 @@ FN is the function to call on click."
(lsp-ui-doc--put-click (match-beginning 0) (match-end 0)
'browse-url-at-mouse)))))

(defun lsp-ui-doc--render-buffer (string symbol)
"Set the buffer with STRING."
(defvar lsp-ui-doc--render-string nil
"The string to render in the documentation popup.")
(defvar lsp-ui-doc--render-symbol nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you need this two global variables? The original version already pass symbol and string, why need to change function signature?

Copy link
Contributor Author

@Sorixelle Sorixelle Jun 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to call lsp-ui-doc--render-buffer from the xwidget-event callback lambda to solve the problem where webkit frames would be empty the first time they popped up.

lsp-ui/lsp-ui-doc.el

Lines 651 to 658 in 61651ba

(when lsp-ui-doc-use-webkit
(define-key (current-global-map) [xwidget-event]
(lambda ()
(interactive)
(let ((xwidget-event-type (nth 1 last-input-event)))
(when (eq xwidget-event-type 'load-changed)
(lsp-ui-doc--render-buffer))

I can't just pass the parameters down to it, because it's only called when the webkit widget is first created, and since it's a lambda, it'll just capture the first values it sees, and won't update on subsequent hovers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to call lsp-ui-doc--render-buffer from the xwidget-event callback lambda to solve the problem where webkit frames would be empty the first time they popped up

Interesting. I wonder how just call lsp-ui-doc--render-buffer will resolve that problem?
From what I can see, the global vars are updated in lsp-ui-doc--display, which in turn will also call lsp-ui-doc--render-buffer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Sorixelle

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is, lsp-ui-doc--render-buffer inserts the text directly into the webkit widget via a javascript call, so it needs to be called after both the webkit widget has been inserted, and after lsp-ui-doc.html has finished loading. That last one in particular isn't guaranteed by the time lsp-ui-doc--display has finished executing, but that 'load-changed event is fired specifically when the page has finished loading, so it's the best thing for us to use, unless there's something else I'm missing.

"The symbol to render documentation for.")

(defun lsp-ui-doc--render-buffer ()
"Set the buffer with `lsp-ui-doc--render-string'."
(lsp-ui-doc--with-buffer
(if lsp-ui-doc-use-webkit
(progn
(lsp-ui-doc--webkit-execute-script
(format
"renderMarkdown('%s', '%s');"
symbol
(url-hexify-string string))
lsp-ui-doc--render-symbol
(url-hexify-string lsp-ui-doc--render-string))
'lsp-ui-doc--webkit-resize-callback))
(erase-buffer)
(let ((inline-p (lsp-ui-doc--inline-p)))
(insert (concat (unless inline-p (propertize "\n" 'face '(:height 0.2)))
(s-trim string)
(s-trim lsp-ui-doc--render-string)
(unless inline-p (propertize "\n\n" 'face '(:height 0.3))))))
(lsp-ui-doc--make-clickable-link))
(setq-local face-remapping-alist `((header-line lsp-ui-doc-header)))
(setq-local window-min-height 1)
(setq header-line-format (when lsp-ui-doc-header (concat " " symbol))
(setq header-line-format (when lsp-ui-doc-header (concat " " lsp-ui-doc--render-symbol))
mode-line-format nil
cursor-type nil)))

Expand Down Expand Up @@ -619,49 +593,66 @@ HEIGHT is the documentation number of lines."
(defun lsp-ui-doc--inline-p ()
"Return non-nil when the documentation should be display without a child frame."
(or (not lsp-ui-doc-use-childframe)
(not (display-graphic-p))
(not (posframe-workable-p))
(not (fboundp 'display-buffer-in-child-frame))))

(defun lsp-ui-doc--display (symbol string)
"Display the documentation."
(setq lsp-ui-doc--render-symbol symbol
lsp-ui-doc--render-string string)
(when (and lsp-ui-doc-use-webkit (not (featurep 'xwidget-internal)))
(setq lsp-ui-doc-use-webkit nil))
(if (or (null string) (string-empty-p string))
(lsp-ui-doc--hide-frame)
(lsp-ui-doc--render-buffer string symbol)
(lsp-ui-doc--render-buffer)
(if (lsp-ui-doc--inline-p)
(lsp-ui-doc--inline)
(unless (lsp-ui-doc--get-frame)
(when (or (not lsp-ui-doc-use-webkit)
(not (lsp-ui-doc--get-frame)))
(lsp-ui-doc--set-frame (lsp-ui-doc--make-frame)))
(unless lsp-ui-doc-use-webkit
(lsp-ui-doc--resize-buffer)
(lsp-ui-doc--move-frame (lsp-ui-doc--get-frame)))
(unless (frame-visible-p (lsp-ui-doc--get-frame))
(make-frame-visible (lsp-ui-doc--get-frame))))))

(defun lsp-ui-doc--posframe-poshandler-point-top-left-corner (info)
"Place the posframe at the top-left corner of the point without covering the
point.

The structure of INFO is defined in the documentation of `posframe-show'."
(let* ((frame (plist-get info :posframe))
(height (frame-pixel-height frame)))
(posframe-poshandler-point-bottom-left-corner info (- height))))

(defun lsp-ui-doc--make-frame ()
"Create the child frame and return it."
(lsp-ui-doc--delete-frame)
(let* ((after-make-frame-functions nil)
(before-make-frame-hook nil)
(name-buffer (lsp-ui-doc--make-buffer-name))
(buffer (get-buffer name-buffer))
(params (append lsp-ui-doc-frame-parameters
`((name . "")
(default-minibuffer-frame . ,(selected-frame))
(minibuffer . ,(minibuffer-window))
(left-fringe . ,(frame-char-width))
(background-color . ,(face-background 'lsp-ui-doc-background nil t)))))
(window (display-buffer-in-child-frame
buffer
`((child-frame-parameters . ,params))))
(frame (window-frame window)))
(let* ((before-make-frame-hook nil)
(buffer-name (lsp-ui-doc--make-buffer-name))
(buffer (get-buffer-create buffer-name))
(params (append lsp-ui-doc-frame-parameters
`((name . "")
(default-minibuffer-frame . ,(selected-frame))
(minibuffer . ,(minibuffer-window)))))
(position (pcase (list lsp-ui-doc-position lsp-ui-doc-alignment)
('(top frame) #'posframe-poshandler-frame-top-right-corner)
('(top window) #'posframe-poshandler-window-top-right-corner)
('(bottom frame) #'posframe-poshandler-frame-bottom-right-corner)
('(bottom window) #'posframe-poshandler-window-bottom-right-corner)
('(at-point frame) #'lsp-ui-doc--posframe-poshandler-point-top-left-corner)
('(at-point window) #'lsp-ui-doc--posframe-poshandler-point-top-left-corner)))
(frame (posframe-show buffer
:width lsp-ui-doc-max-width
:height lsp-ui-doc-max-height
:poshandler position
:internal-border-width 1
:internal-border-color lsp-ui-doc-border
:left-fringe t
:right-fringe t
:background-color (face-background 'lsp-ui-doc-background nil t)
:override-parameters params))
(window (frame-root-window frame)))
(with-current-buffer buffer
(lsp-ui-doc-frame-mode 1))
(set-frame-parameter nil 'lsp-ui-doc-buffer buffer)
(set-window-dedicated-p window t)
(redirect-frame-focus frame (frame-parent frame))
(set-face-background 'internal-border lsp-ui-doc-border frame)
(lsp-ui-doc-frame-mode 1)
(visual-line-mode 1))
(set-face-background 'fringe nil frame)
(run-hook-with-args 'lsp-ui-doc-frame-hook frame window)
(when lsp-ui-doc-use-webkit
Expand All @@ -670,6 +661,8 @@ HEIGHT is the documentation number of lines."
(interactive)

(let ((xwidget-event-type (nth 1 last-input-event)))
(when (eq xwidget-event-type 'load-changed)
(lsp-ui-doc--render-buffer))
;; (when (eq xwidget-event-type 'load-changed)
;; (lsp-ui-doc--move-frame (lsp-ui-doc--get-frame)))

Expand Down Expand Up @@ -727,7 +720,7 @@ BUFFER is the buffer where the request has been made."
(defun lsp-ui-doc--delete-frame ()
"Delete the child frame if it exists."
(-when-let (frame (lsp-ui-doc--get-frame))
(delete-frame frame)
(posframe-delete-frame (lsp-ui-doc--make-buffer-name))
(lsp-ui-doc--set-frame nil)))

(defun lsp-ui-doc--visible-p ()
Expand Down Expand Up @@ -766,6 +759,27 @@ before, or if the new window is the minibuffer."
(and (buffer-live-p it) it)
(kill-buffer it)))

(define-minor-mode lsp-ui-doc-frame-mode
"Marker mode to add additional key bind for lsp-ui-doc-frame."
:init-value nil
:lighter ""
:group lsp-ui-doc
:keymap `(([?q] . lsp-ui-doc-unfocus-frame)))

(defun lsp-ui-doc-focus-frame ()
"Focus into lsp-ui-doc-frame."
(interactive)
(when (lsp-ui-doc--frame-visible-p)
(lsp-ui-doc--with-buffer
(setq cursor-type t))
(select-frame-set-input-focus (lsp-ui-doc--get-frame))))

(defun lsp-ui-doc-unfocus-frame ()
"Unfocus from lsp-ui-doc-frame."
(interactive)
(when-let ((frame (frame-parent (lsp-ui-doc--get-frame))))
(select-frame-set-input-focus frame)))

(define-minor-mode lsp-ui-doc-mode
"Minor mode for showing hover information in child frame."
:init-value nil
Expand All @@ -783,11 +797,17 @@ before, or if the new window is the minibuffer."
(cl-callf copy-tree frameset-filter-alist)
(push '(lsp-ui-doc-frame . :never) frameset-filter-alist)))
(add-hook 'post-command-hook 'lsp-ui-doc--make-request nil t)
(add-hook 'delete-frame-functions 'lsp-ui-doc--on-delete nil t))
(add-hook 'delete-frame-functions 'lsp-ui-doc--on-delete nil t)
(advice-add #'posframe--redirect-posframe-focus
:before-until (lambda (&rest _)
lsp-ui-doc-frame-mode)
'((name . lsp-ui-doc--dont-redirect-posframe))))
(t
(lsp-ui-doc-hide)
(remove-hook 'post-command-hook 'lsp-ui-doc--make-request t)
(remove-hook 'delete-frame-functions 'lsp-ui-doc--on-delete t))))
(remove-hook 'delete-frame-functions 'lsp-ui-doc--on-delete t)
(advice-remove #'posframe--redirect-posframe-focus
'lsp-ui-doc--dont-redirect-posframe))))

(defun lsp-ui-doc-enable (enable)
"Enable/disable ‘lsp-ui-doc-mode’.
Expand Down Expand Up @@ -825,26 +845,5 @@ It is supposed to be called from `lsp-ui--toggle'"
(cancel-timer lsp-ui-doc--unfocus-frame-timer))
(add-hook 'post-command-hook 'lsp-ui-doc--glance-hide-frame))

(define-minor-mode lsp-ui-doc-frame-mode
"Marker mode to add additional key bind for lsp-ui-doc-frame."
:init-value nil
:lighter ""
:group lsp-ui-doc
:keymap `(([?q] . lsp-ui-doc-unfocus-frame)))

(defun lsp-ui-doc-focus-frame ()
"Focus into lsp-ui-doc-frame."
(interactive)
(when (lsp-ui-doc--frame-visible-p)
(lsp-ui-doc--with-buffer
(setq cursor-type t))
(select-frame-set-input-focus (lsp-ui-doc--get-frame))))

(defun lsp-ui-doc-unfocus-frame ()
"Unfocus from lsp-ui-doc-frame."
(interactive)
(when-let ((frame (frame-parent (lsp-ui-doc--get-frame))))
(select-frame-set-input-focus frame)))

(provide 'lsp-ui-doc)
;;; lsp-ui-doc.el ends here
2 changes: 1 addition & 1 deletion lsp-ui.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
;; Author: Sebastien Chapuis <sebastien@chapu.is>, Fangrui Song <i@maskray.me>
;; Keywords: languages, tools
;; URL: https://github.com/emacs-lsp/lsp-ui
;; Package-Requires: ((emacs "26.1") (dash "2.14") (dash-functional "1.2.0") (lsp-mode "6.0") (markdown-mode "2.3"))
;; Package-Requires: ((emacs "26.1") (dash "2.14") (dash-functional "1.2.0") (lsp-mode "6.0") (markdown-mode "2.3") (posframe "0.7.0"))
;; Version: 6.3

;;; License
Expand Down