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

Fix text align for buffer-face-mode #20

Merged
merged 3 commits into from
Mar 19, 2024
Merged

Fix text align for buffer-face-mode #20

merged 3 commits into from
Mar 19, 2024

Conversation

chuxubank
Copy link
Contributor

@chuxubank chuxubank commented Mar 18, 2024

When we use buffer-face-mode and set different font other than the default font, the text align of sideline is incorrect, like this:
image

After this fix, it would looks like:
image

See also: emacs-lsp/lsp-ui#184

chuxubank added a commit to chuxubank/cat-emacs that referenced this pull request Mar 18, 2024
@jcs090218
Copy link
Member

jcs090218 commented Mar 18, 2024

This solution doesn't look right since the length function isn't accurate enough for wider characters. 🤔

And it doesn't work on my machine:

2024-03-18 12 02 50

@jcs090218 jcs090218 added the bug Something isn't working label Mar 18, 2024
@chuxubank
Copy link
Contributor Author

chuxubank commented Mar 19, 2024

Wow, it seemed you are using variable pitch font for programming, thats cool.
I will try to fix this :P

The length only calculate the string's length, the issue is the (window-font-width) do not work if the font's width is not fixed. Seemed not easy to fix it.
(One possible fix could change the sideline's font to a mono font.)

I will keep this PR open and use the modified version in my Emacs config for now.

@jcs090218
Copy link
Member

Wow, it seemed you are using variable pitch font for programming, thats cool.

No, those are the default for buffer-face-mode on Windows. I use Ubuntu Mono by default.

The length only calculate the string's length, the issue is the (window-font-width) do not work if the font's width is not fixed. Seemed not easy to fix it.
(One possible fix could change the sideline's font to a mono font.)

Yes, this is not trivial. I have spent countless hours but still haven't found the ideal solution. 😓

@chuxubank
Copy link
Contributor Author

chuxubank commented Mar 19, 2024

No, those are the default for buffer-face-mode on Windows. I use Ubuntu Mono by default.

I see, I think my fix should work for mono fonts when using buffer-face-mode, and it also benefit the text-scale-mode

I am using Iosevka as default font and use Victor Mono for programming.

In emacs-lsp/lsp-ui#184 (comment) there's a function to do with the variable pitch font but kind of heavy.

I think my fix is enough for most cases when using mono fonts.

@gustavotcabral
Copy link

No, those are the default for buffer-face-mode on Windows. I use Ubuntu Mono by default.

I see, I think my fix should work for mono fonts when using buffer-face-mode, and it also benefit the text-scale-mode

I am using Iosevka as default font and use Victor Mono for programming.

In emacs-lsp/lsp-ui#184 (comment) there's a function to do with the variable pitch font but kind of heavy.

I think my fix is enough for most cases when using mono fonts.

I've been using my fix since then (Ubuntu 22.04, using Wayland and variable-width fonts), flawlessy. I didn't even know this bug hadn't been fixed yet.

variable_width

@jcs090218
Copy link
Member

jcs090218 commented Mar 19, 2024

and it also benefit the text-scale-mode

How about something like?

(defun sideline--align-right (str offset)
  "Align sideline STR from the right of the window.

Argument OFFSET is additional calculation from the right alignment."
  (let ((graphic-p (display-graphic-p))
        (fringes (window-fringes)))
    (list (+
           ;; If the sideline text is displayed without at least 1 pixel gap from the right fringe and
           ;; overflow-newline-into-fringe is not true, emacs will line wrap it.
           (if (and graphic-p
                    (> (nth 1 fringes) 0)
                    (not overflow-newline-into-fringe))
               1
             0)
           (* (window-font-width)
              (+ offset (if graphic-p
                            ;; If right fringe deactivated add 1 offset
                            (if (= 0 (nth 1 fringes)) 1 0)
                          1)
                 (sideline--str-len str)))))))

I think my fix is enough for most cases when using mono fonts.

It won't work if you have wide characters. The above patch should fix this as well.

@chuxubank
Copy link
Contributor Author

This version works well for me. Thanks!
I will update this PR.

@jcs090218 jcs090218 merged commit 04a525f into emacs-sideline:master Mar 19, 2024
11 of 12 checks passed
@jcs090218
Copy link
Member

Thank you!

@chuxubank chuxubank deleted the patch-1 branch March 19, 2024 03:17
chuxubank added a commit to chuxubank/cat-emacs that referenced this pull request Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants