rougier / nano-modeline

GNU Emacs / N Λ N O Modeline
GNU General Public License v3.0
170 stars 29 forks source link

Replace `(length` with `(string-width` #74

Open TomoeMami opened 6 months ago

TomoeMami commented 6 months ago

Improve multibyte characters align.

Ref: https://github.com/rougier/nano-modeline/issues/72

rougier commented 6 months ago

I think string-pixel-width might be more accurate

aaronjensen commented 6 months ago

It might, but I believe that is incredibly slow if I’m not mistaken. That would be a non-starter for a modeline.

aaronjensen commented 6 months ago

Also, I think it’s fair to say that using a monospace font for any of these things is a requirement. We shouldn’t expect pixel perfect alignment of variable width fonts in Emacs yet.

TomoeMami commented 6 months ago

It might, but I believe that is incredibly slow if I’m not mistaken. That would be a non-starter for a modeline.

And string-pixel-width will be changed when users specified their own font size.

https://github.com/rougier/nano-modeline/issues/72#issuecomment-1952521732

rougier commented 6 months ago

Is is really that slow ?

aaronjensen commented 6 months ago

~16x slower on my machine, but still relatively fast (it takes a bit over 100 of them to get to 1ms). If there is only a few done it may not matter. I still try to avoid anything that is remotely slow in modelines because it affects typing and scrolling latency, and bad modelines have actually had material impact on my experience in Emacs. Modelines are rendered a lot, so for anything that accesses the file system, they benefit from memoization, etc.

string-pixel-width actually creates a buffer, puts the string in it, then measures it using buffer-text-pixel-size which actually renders the text in a temporary window. I'd personally avoid it in things like modelines, or make it optional so someone who needed it could opt in (which adds complexity)

rougier commented 6 months ago

16x is huge slowdown. I guess we can wait for emacs 30.0 and the mode-line-format-right-align.

aaronjensen commented 6 months ago

To be clear I was just benchmarking the function in isolation, not the entire modeline.

aaronjensen commented 6 months ago

I vaguely remember that the right align thing doesn’t take variable fonts into account but I would have to test it.