emacs-lsp / lsp-ui

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

Sideline overflows current line with bigger font size #184

Open infinisil opened 6 years ago

infinisil commented 6 years ago

When I increase the font size with C-x C-+, the sideline will sometimes overflow the line onto the next one:

screenshot_3

Very annoying, because when moving over such lines while coding, half of the screen gets thrown around because of the additional line(s).

yyoncho commented 4 years ago

Should be fixed with https://github.com/emacs-lsp/lsp-ui/pull/475

infinisil commented 4 years ago

Even with latest master it doesn't appear to be fully fixed. While it now takes more than just one C-x C-+, it still overflows after a bunch: 2020-07-24_18-45

yyoncho commented 4 years ago

Can you try turning off line numbers?

infinisil commented 4 years ago

Oh that apparently makes it worse. I was using global-display-line-numbers-mode for reference. Turning it off, with the standard font size: 2020-07-24_20-29

Then doing C-x C-+ once: 2020-07-24_20-29_1


In comparison, here is the same sequence with global-display-line-numbers-mode turned on: Standard font size: 2020-07-24_20-31

Note how the text isn't flush on the right side anymore (maybe a clue as to what's going wrong)

Then doing C-x C-+ once: 2020-07-24_20-32

Note how the text doesn't overflow yet (but it does with more C-x C-+), but it got a bit closer to the righthand edge.

sebastiencs commented 4 years ago

@Infinisil It should be fixed with https://github.com/emacs-lsp/lsp-ui/commit/3686ed86ac37c74bfdf2e05f96c968d082b826e7

infinisil commented 4 years ago

Can confirm, thanks!

renatoathaydes commented 3 years ago

I am currently seeing the same thing even after upgrading all my packages to the latest versions.

Screen Shot 2021-08-21 at 11 15 34

I don't have a lot of packages installed, and this makes it impossible to write anything as text jumps around too much (unless disabling lsp-ui).

Can you tell me if I need to configure something to change this?

siraben commented 2 years ago

I can also replicate this.

Screen Shot 2021-12-26 at 8 10 13 PM
bmathieu33 commented 2 years ago

I have this problem too, and I think I found what triggers this on my configuration.

First of all: for prog-mode I set another font using buffer-face-mode:

(defun my/prog-mode-face ()
  "Set buffer face for source code."
  (set (make-local-variable 'buffer-face-mode-face)
       '(:family "Fira Code"))
  (buffer-face-mode t)
  )

For the record buffer-face-mode and text-scale-* functions are all defined in face-remap.el.

When buffer face mode is active lsp-ui-sideline--window-width will find a value too high for current font:

I have not customized the default font. I am running this version (Ubuntu 21.10, using wayland):

GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.23, cairo version 1.16.0) of 2021-01-18, modified by Debian
sam-masaki commented 2 years ago

I have this problem specifically with the font Iosevka. Setting the font to the default width Iosevka causes this, but using the wider Extended version works fine. Maybe this is also an issue with font width or font aspect ratio.

Edit: Looking into it a bit more, in my case one of the faces the sideline text uses is a markdown face that inherits from fixed-pitch which uses the system default monospace font. It's the difference in width between that font and the default font that was causing my problem. Looking closer at the above screenshots, they seem to all have the same font between the sideline and the main text, so the problem I had is a little different.

The spot fix for anyone with the same issue is just setting the font family for fixed-pitch.

runejuhl commented 2 years ago

Thanks a bunch @sam-masaki -- setting the font for the fixed-pitch face to Iosevka (same as my regular font) fixed my issues. 🎉

For anyone not well versed in emacs this can be done with M-x customize-face, selecting fixed-pitch and changing the value. To see your default font check the value for the default face.

gustavotcabral commented 2 years ago

Same here. I've fixed it changing the function lsp-ui-sideline--align to return (cons oldresult 'width). I'm using Emacs 28.1.

Reference: https://www.gnu.org/software/emacs/manual/html_node/elisp/Pixel-Specification.html

(defun lsp-ui-sideline--align (&rest lengths)
  "Align sideline string by LENGTHS from the right of the window."
  (cons (+ (apply '+ lengths)
           (if (display-graphic-p) 1 2))
        'width))

emacs_normal

emacs_increased

mugijiru commented 2 years ago

@gustavotcabral Great! I tried your code and it solved the problem in my environment. Thank you!

jcs090218 commented 2 years ago

I think the correct fix is

(defun sideline--compute-height ()
  "Return a fixed size for text in sideline."
  (if (null text-scale-mode-remapping)
      '(height 1)
    ;; Readjust height when text-scale-mode is used
    (list 'height
          (/ 1 (or (plist-get (cdr text-scale-mode-remapping) :height)  ; should remove this
                   (plist-get (cdar text-scale-mode-remapping) :height)  ; add this
                   1)))))
gustavotcabral commented 2 years ago

I think the correct fix is

(defun sideline--compute-height ()
  "Return a fixed size for text in sideline."
  (if (null text-scale-mode-remapping)
      '(height 1)
    ;; Readjust height when text-scale-mode is used
    (list 'height
          (/ 1 (or (plist-get (cdr text-scale-mode-remapping) :height)  ; should remove this
                   (plist-get (cdar text-scale-mode-remapping) :height)  ; add this
                   1)))))

Hi, @jcs090218.

I think this function is unnecessary (and it has a "minor" bug: when text-scale-mode is used after the overlay creation, the position won't be fixed automatically).

Using customize-face, we can achieve both fixed height (your approach) and scalable height of the sideline text (see #561 and #580).

My "perfect" solution until now (for Emacs 28.1 with Cairo; it doesn't work for 27.2):

(defun lsp-ui-sideline--align (&rest lengths)
  "Align sideline string by LENGTHS from the right of the window."
  (cons (+ (apply '+ lengths)
           (if (display-graphic-p) 1 2))
        'width))

(defun lsp-ui-sideline--compute-height ()
  "Return a fixed size for text in sideline."
  nil)

;; ...and set the face of the " " prefixes to the same face of the text.

It even works on "live" face customization, but it doesn't work with variable-width fonts.

sideline_fixed

sideline_scaled

jcs090218 commented 2 years ago

Hmmm... This is what I get from eval your solution: 😕

(defun lsp-ui-sideline--align (&rest lengths)
  "Align sideline string by LENGTHS from the right of the window."
  (cons (+ (apply '+ lengths)
           (if (display-graphic-p) 1 2))
        'width))

(defun lsp-ui-sideline--compute-height ()
  "Return a fixed size for text in sideline."
  nil)

Image 1

This is on Windows, with Emacs 29.0.50.

My "perfect" solution until now (for Emacs 28.1 with Cairo; it doesn't work for 27.2):

We have to find a generic way to resolve this issue since this package should support 26.1 or above.

bmathieu33 commented 2 years ago

I don't know if this is the same problem that I face (but it did look like screenshots posted by @infinisil when creating this issue). Following my comment, here is a patch that fixed problems for me. Mainly it uses hopefully appropriate functions to get actual available width. I don't have line overflows since.

--- a/lsp-ui-sideline.el
+++ b/lsp-ui-sideline.el
@@ -296,8 +296,9 @@ MARKED-STRING is the string returned by `lsp-ui-sideline--extract-info'."

 (defun lsp-ui-sideline--align (&rest lengths)
   "Align sideline string by LENGTHS from the right of the window."
-  (+ (apply '+ lengths)
-     (if (display-graphic-p) 1 2)))
+  (list (* (window-font-width)
+           (+ (apply '+ lengths)
+              (if (display-graphic-p) 1 2)))))

 (defun lsp-ui-sideline--compute-height ()
   "Return a fixed size for text in sideline."
@@ -358,7 +359,7 @@ is set to t."
        0)))

 (defun lsp-ui-sideline--window-width ()
-  (- (min (window-text-width) (window-body-width))
+  (- (window-max-chars-per-line)
      (lsp-ui-sideline--margin-width)
      (or (and (>= emacs-major-version 27)
               ;; We still need this number when calculating available space
@@ -381,7 +382,7 @@ is set to t."
   (when (and (lsp-ui-sideline--valid-tag-p tag 'line)
              (not (lsp-ui-sideline--stop-p)))
     (let ((inhibit-modification-hooks t)
-          (win-width (window-body-width))
+          (win-width (lsp-ui-sideline--window-width))
           ;; sort by bounds
           (list-infos (--sort (< (caadr it) (caadr other)) list-infos)))
       (lsp-ui-sideline--delete-kind 'info)

If you want to try it more easily here is the functions as currently defined in my use-package :config section for lsp-ui:

    (defun lsp-ui-sideline--window-width ()
      (- (window-max-chars-per-line)
         (lsp-ui-sideline--margin-width)
         (or (and (>= emacs-major-version 27)
                  ;; We still need this number when calculating available space
                  ;; even with emacs >= 27
                  (lsp-ui-util-line-number-display-width))
             0)))

    (defun lsp-ui-sideline--display-all-info (list-infos tag bol eol)
      (when (and (lsp-ui-sideline--valid-tag-p tag 'line)
                 (not (lsp-ui-sideline--stop-p)))
        (let ((inhibit-modification-hooks t)
              (win-width (lsp-ui-sideline--window-width))
              ;; sort by bounds
              (list-infos (--sort (< (caadr it) (caadr other)) list-infos)))
          (lsp-ui-sideline--delete-kind 'info)
          (--each list-infos
            (-let (((symbol bounds info) it))
              (lsp-ui-sideline--push-info win-width symbol bounds info bol eol))))))

    (defun lsp-ui-sideline--align (&rest lengths)
      (list (* (window-font-width)
               (+ (apply '+ lengths) (if (display-graphic-p) 1 2)))))
jcs090218 commented 2 years ago

@bmathieu33 Thanks for the patch! I have tested it on my end and it works!

But since I couldn't reproduce this, I would like to have other people test it! @gustavotcabral @mugijiru

mugijiru commented 2 years ago

@bmathieu33 @jcs090218 Thanks for the patch and mention. I tested with the patch. However, line overflow still occurs.

image

This capture's text-scale-mode-amount is 3 and my (emacs-version) is

GNU Emacs 28.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.33, cairo version 1.17.6) of 2022-04-28

I hope this information is of some help to you.

mugijiru commented 2 years ago

Sorry, I had overlooked the lsp-ui-sideline--align change. I also eval lsp-ui-sideline--align and tested again. And confirmed that it is fixed! Thanks!

image

mugijiru commented 2 years ago

When I imported the latest version of lsp-ui and tested it, I got the following display.

image

The sideline uses a small font and has unnecessary margins on the right side. So I think lsp-ui-sideline--compute-height should always return 1

jcs090218 commented 2 years ago

Can you test with text-scale-increase or text-scale-decrease? Sorry, I am still trying to collect as much information as possible in order to come up with the correct fix. 😓

Edit: maybe with the different font size as well.

mugijiru commented 2 years ago

I will provide as much information as I can.

I was testing with text-scale-increase. The following is the result of testing with text-scale-decrease in addition to it.

Latest version (ca195f3ba)

No text-scale changes

image

execute text-scale-increase 1 time

image

execute text-scale-increase 2 times

image

execute text-scale-increase 3 times

image

execute text-scale-decrease 1 time

image

execute text-scale-decrease 2 times

image

execute text-scale-decrease 3 times

image

Latest version (ca195f3b) with bmathieu33's patch

No text-scale changes

image

execute text-scale-increase 1 time

image

execute text-scale-increase 2 times

image

execute text-scale-increase 3 times

In this case, the sideline is not displayed

image

execute text-scale-decrease 1 time

image

execute text-scale-decrease 2 times

image

execute text-scale-decrease 3 times

image

Latest version (ca195f3b) with bmathieu33's patch and lsp-ui-sideline--compute-height always return 1

I am changing lsp-ui-sideline--compute-height as follows:

(defun lsp-ui-sideline--compute-height ()
  1)

No text-scale changes

image

execute text-scale-increase 1 time

image

execute text-scale-increase 2 times

image

execute text-scale-increase 3 times

image

execute text-scale-decrease 1 time

image

execute text-scale-decrease 2 times

image

execute text-scale-decrease 3 times

image


And my font config is:

(let* ((size 18)
       (asciifont "Ricty Diminished")      ; ASCII fonts
       (jpfont "Ricty Diminished")         ; Japanese fonts
       (h (* size 10))
       (fontspec (font-spec :family asciifont))
       (jp-fontspec (font-spec :family jpfont)))
  (set-face-attribute 'default nil :family asciifont :height h)
  (set-fontset-font nil 'japanese-jisx0213.2004-1 jp-fontspec)
  (set-fontset-font nil 'japanese-jisx0213-2 jp-fontspec)
  (set-fontset-font nil 'katakana-jisx0201 jp-fontspec)
  (set-fontset-font nil '(#x0080 . #x024F) fontspec)
  (set-fontset-font nil '(#x0370 . #x03FF) fontspec))
gustavotcabral commented 2 years ago

Hmmm... This is what I get from eval your solution: confused

(defun lsp-ui-sideline--align (&rest lengths)
  "Align sideline string by LENGTHS from the right of the window."
  (cons (+ (apply '+ lengths)
           (if (display-graphic-p) 1 2))
        'width))

(defun lsp-ui-sideline--compute-height ()
  "Return a fixed size for text in sideline."
  nil)

Image 1

This is on Windows, with Emacs 29.0.50.

My "perfect" solution until now (for Emacs 28.1 with Cairo; it doesn't work for 27.2):

We have to find a generic way to resolve this issue since this package should support 26.1 or above.

@jcs090218 I can confirm this on Windows with TTF fonts. But OTF fonts seem to work well. Could you repeat the test using OpenType fonts?

When I imported the latest version of lsp-ui and tested it, I got the following display.

image

The sideline uses a small font and has unnecessary margins on the right side. So I think lsp-ui-sideline--compute-height should always return 1

@mugijiru I think you need to propertize the " " prefixes using the same face of the sideline, like I said in my previous comment.

To be clear: what we are doing here is getting around Emacs bugs across versions/platforms in width calculation. We should report those bugs to the Emacs team.

Let's me show another approach:

Pros

  1. Works with fixed and scalable font sizes (via customize-face);
  2. Works with every font I've tested (even variable-width fonts, TrueType and OpenType);
  3. Tested on Windows (Emacs 28.1) and Linux (28.1 and 27.2).

Cons

  1. Doesn't work on "live" face/text-scale updates (the proposed solutions don't work, either);
  2. May be a bit heavy, I believe: it creates a new buffer to compute the text widths.

I use this new function to compute the width:

(defun lsp-ui-sideline--compute-text-width (text-with-properties &optional window)
  (let ((window (or window (selected-window)))
        (remapping-alist face-remapping-alist))
    (with-temp-buffer
      (setq-local face-remapping-alist remapping-alist)
      (set-window-buffer window (current-buffer))
      (insert text-with-properties)
      (car (window-text-pixel-size)))))

I've committed all the changes to my forked repo. I haven't heavily tested it, though.

Screenshot on Windows (using Calibri Light)

calibri

Screenshot on Linux (using distinct font sizes)

lufga_linux

kcbanner commented 2 years ago

I've also had this issue (Windows, Emacs 29), and testing with @gustavotcabral fork seems to fix the issue for me. Thanks!