Cleanup and some drawing improvements.

schriftgestalt commented 3 years ago

I started this to save some space next to the notch. this is the network and CPU item in the taller menu:

yujitach commented 3 years ago

Thanks. I have two comments:

I understand your desire to unify whitespaces, coding styles, absence/presence of (float) or …f for CGFloat which is double on 64 bit architecture. I often think I should do so too! But they make it very hard for me to check what is actually meaningfully changed. If possible, please split the changes to two parts, one which is purely cosmetic and another which does actual changes. At least, please do not combine two different types of changes when you make a pull request the next time.
Your code which checks for the taller menu bar won’t correctly work when the MacBook with a notch is connected to an external display, which retains a shorter menu bar, see the screenshot provided at https://github.com/yujitach/MenuMeters/pull/256#issuecomment-953699976

schriftgestalt commented 3 years ago

I feared you say this. I tried to separate the commits as good as possible into purely whitespace/formatting and functional changes. The actual changes I made are too intertwined and I don’t have time to redo them all. I would have loved to be able to do individual pull requests. Have you tried to split interdependent changes into two branches? But my plan wasn’t to clean up years old code, but to fix a few things to make it nicer.

Is there any 32 but build target? I only changed float to CGFloat when the code interacted with methods that expect them (NSMakePoint). And removing all those pesky f makes (afaik) no difference (again a lot of them where in NSColor methods and they are 64 bit anyway). And the compiler is more clever to cast them properly or use the right type to begin with compared to the times where we needed to cross compile 32 and 64 bit.

The code with the menu bar height should works. It should report the height of the actual menubar. So if you plug in a screen and move the menubar to it, it should return the new value. I know that the button image has the same height on both configurations. My code only changed the padding inside the button. But I can test that more.

schriftgestalt commented 2 years ago

I'm redoing the pull request. Hope the new one will be easier to accept.

yujitach commented 2 years ago

Sorry, I am having some stomach problem in the last couple of days, so my reply has been sporadic. You seem to have more time than me. Do you want to take over being the main maintainer of MenuMeters? Then you can do as you wish. (You would need to learn how to code-sign and notarize the app properly, which would be a pain, though.)

schriftgestalt commented 2 years ago

Full maintainer would be be a bit much right now. But you could make me a collaborator in the repo ;)

yujitach / MenuMeters

Cleanup and some drawing improvements. #259