emacs-lsp / lsp-ui

UI integrations for lsp-mode
https://emacs-lsp.github.io/lsp-ui
GNU General Public License v3.0
1.03k stars 141 forks source link

lsp-ui-doc childframe does not respect altered lsp-ui-doc-background #369

Open svenssonaxel opened 4 years ago

svenssonaxel commented 4 years ago

To reproduce:

svenssonaxel commented 4 years ago

The following diff is a first attempt to fix it. It works both inline and with childframe. Note that

diff --git a/lsp-ui-doc.el b/lsp-ui-doc.el
index 210f5c8..118c7dd 100644
--- a/lsp-ui-doc.el
+++ b/lsp-ui-doc.el
@@ -313,8 +313,7 @@ We don't extract the string that `lps-line' is already displaying."
   "Set background color of the WebKit widget."
   (lsp-ui-doc--webkit-execute-script
    (format "document.body.style.background = '%s';"
-           "#fdfdfd"
-           ;; (face-attribute 'lsp-ui-doc-background :background)
+           (face-attribute 'lsp-ui-doc-background :background)
            )))

 (defun lsp-ui-doc--webkit-set-foreground ()
@@ -530,7 +529,7 @@ FN is the function to call on click."

 (defun lsp-ui-doc--inline-padding (string len)
   (let ((string (concat " " string (make-string (- len (string-width string)) ?\s) " ")))
-    (add-face-text-property 0 (length string) (list :background (face-background 'lsp-ui-doc-background nil t)) t string)
+    (add-face-text-property 0 (length string) 'lsp-ui-doc-background t string)
     string))

 (defun lsp-ui-doc--inline-faking-frame (doc-strings)
@@ -783,5 +782,15 @@ It is supposed to be called from `lsp-ui--toggle'"
   (lsp-ui-doc-show)
   (add-hook 'pre-command-hook 'lsp-ui-doc--glance-hide-frame))

+(defun lsp-ui-doc-update-frame-background (&rest _ignore)
+  (if-let (frame (lsp-ui-doc--get-frame))
+      (if  lsp-ui-doc-use-webkit
+          (lsp-ui-doc--webkit-set-background)
+        (set-frame-parameter
+         frame
+         'background-color
+         (face-background 'lsp-ui-doc-background nil t)))))
+(advice-add 'custom-set-faces :after #'lsp-ui-doc-update-frame-background)
+
 (provide 'lsp-ui-doc)
 ;;; lsp-ui-doc.el ends here
danielmartin commented 4 years ago

Thanks for the patch. Could you create a separate PR with it so that we can review/test it and eventually merge it?

svenssonaxel commented 4 years ago

@danielmartin PR created.