sideline icon indicating copy to clipboard operation
sideline copied to clipboard

Fix right align with variable width fonts

Open chuxubank opened this issue 2 years ago • 7 comments

When fixing #20 , we use current window's font width to calculate the string width, but for variable width fonts, which usually used in variable-pitch-mode, this will cause error.

Whereas emacs's built-in string-pixel-width do not respect the face remapping, which cause wrong string width when use buffer-face-mode.

This pr mixed the @gustavotcabral 's patch and emac's built-in string-pixel-width to directly get the correct string width.

This fix is not that heavy because we are doing almost the same thing as emac's built-in string-pixel-width.

Some screenshots after fix: image image

chuxubank avatar Apr 03 '24 14:04 chuxubank

The only issue with this patch is the space calculation needs to be corrected. Here are the conditions we want to consider:

  1. Make sure it works with text-scale-mode
  2. Make sure it works with buffer-face-mode
  3. line space calculation is correct (space enough to display sideline information)

🤔

jcs090218 avatar Apr 03 '24 21:04 jcs090218

For 1 and 2, I tested and just works well. BTW, variable-pitch-mode is kind of buffer-face-mode. For 3, it does have some problem. When text-scale-mode is increasing, the space calculating seemed not doing the right thing, and may cause overflow when align left. image I will try to fix it.

chuxubank avatar Apr 04 '24 01:04 chuxubank

~~It seemed that only if I change the sideline--str-len to this:~~

(defun sideline--str-len (str)
  "Calculate STR length."
  (length str))

~~Then all things works for me?~~ image image

Sorry, I forget to test align left.

chuxubank avatar Apr 04 '24 01:04 chuxubank

It seemed that only if I change the sideline--str-len to this:

I think you got it! TBH, I don't remember how I created this since it's been a while. 😓 Can you apply the changes to this PR? Thanks!

jcs090218 avatar Apr 04 '24 02:04 jcs090218

Sorry, I forgot to test align left.

Yeah, this project got a little complicated. I seldom forget things I need to test. Please make sure you have sidelines on the same line (one on each side), so they don't conflict with each other. 👍

jcs090218 avatar Apr 04 '24 02:04 jcs090218

I finally fixed the space calculation by using (window-font-width) instead of (frame-char-width) in sideline--str-len Test Result: image image

@jcs090218 Could can have a check with my new commit?

chuxubank avatar Apr 27 '24 04:04 chuxubank

Sorry I find out that the left align fix is done by your https://github.com/emacs-sideline/sideline/commit/c1729b2b9d2ca6b37bf605ca2271e570f30316f0 Simply replace the sideline--str-len with length is enough. Test Result: image image Works better than (window-font-width)solution.

@jcs090218 Should I delete the sideline--str-len function and replace it with length in the code or simply keep it as is?

chuxubank avatar Apr 27 '24 05:04 chuxubank

Does the latest commit works for you? See a4626181434534385fbb199fa17d606f89e86e7c. 🤔

jcs090218 avatar May 10 '24 02:05 jcs090218

Does the latest commit works for you? See a462618. 🤔

Yes, as long as we respect the face-remapping-alist then it should works!

image

Thanks for the quick fix, I will close this PR.

chuxubank avatar May 10 '24 03:05 chuxubank