Reintroduce analogue clock time icon

seagle0128 / doom-modeline

A fancy and fast mode-line inspired by minimalism design.

https://seagle0128.github.io/doom-modeline/

GNU General Public License v3.0

1.29k stars 159 forks source link

Reintroduce analogue clock time icon #700

Closed tecosaur closed 7 months ago

tecosaur commented 7 months ago

Take 2 of #698, resolving the issue noted in https://github.com/seagle0128/doom-modeline/pull/698#discussion_r1515654446.

Now works on my machine without me loading my config :wink:.

seagle0128 commented 7 months ago

Below is the screenshot in my env. The clock looks wired.

tecosaur commented 7 months ago

Interesting. Seems like the sizing is worse for some reason? It might be the case that the size factor might want to be tweaked depending on the particular graphical/modeline configuration.

Would you like me to expose a clock size customisation variable (and let me know if you'd prefer a defcustom or defvar if we're just testing)?

seagle0128 commented 7 months ago

I set doom-modeline--clock-inverse-size to 3.0 and it looks great.

It seems related to the font. If the analogue clock is enabled by default, the users have to set this variable. The experience is not good.

tecosaur commented 7 months ago

Ok, so sounds like we should have it off by default.

How about I push these changes?

Change the default value of doom-modeline-time-analogue-clock to nil (now I look I think I may have missed the other non-segment files when copying over the old state, so I'll correct that along the way)
Turn doom-modeline--clock-inverse-size into a new defcustom mentioned in the docstring of doom-modeline-time-analogue-clock, possibly make it the non-inverse size, so it's a bit easier to explain. I just did inverse originally so the range of "good numbers" was nicer (3-5) but 0.2-0.4 doesn't seem to bad.

seagle0128 commented 7 months ago

Sounds good! But I am wondering how to calculate./tweak the ratios/size?

tecosaur commented 7 months ago

Yea, it would be ideal to have something that "just works". I'm not sure how we can do this robustly though?

seagle0128 commented 7 months ago

Change the default value of doom-modeline-time-analogue-clock to nil (now I look I think I may have missed the other non-segment files when copying over the old state, so I'll correct that along the way)

Turn doom-modeline--clock-inverse-size into a new defcustom mentioned in the docstring of doom-modeline-time-analogue-clock, possibly make it the non-inverse size, so it's a bit easier to explain. I just did inverse originally so the range of "good numbers" was nicer (3-5) but 0.2-0.4 doesn't seem to bad.

Please move forward. And it's better merge #689 into this. Then I just need to merge one.

tecosaur commented 7 months ago

Aha! I think I might have figured it out, the problem is that scaling is applied to the SVG based on an (image-compute-scaling-factor image-scaling-factor) call that occurs within create-image (on my system, this results in a scaling factor of ~1.41).

I think we might be able to avoid the extra variable after all!

Edit: on seconds thoughts, it's still worth having, it's just easier to understand how it works now.

tecosaur commented 7 months ago

Let me know how that seems to you.

Since I think this fixes the "weird scaling" issue, I've left doom-modeline-time-analogue-clock as t by default, but happy to change if you'd prefer.

tecosaur commented 7 months ago

I've just made the sizing variable doom-modeline-time-clock-size, and made it so it can be set to an integer number of pixels, or a proportion to `doom-modeline-height'.

seagle0128 commented 7 months ago

Thank you! And I committed a patch 4d1d013 to fix the cache issue. Please check.