sfsam / Itsycal

Itsycal is a tiny calendar for your Mac's menu bar. http://www.mowglii.com/itsycal
MIT License
3.25k stars 234 forks source link

Show ISO dates passed to itsycal via a URL #233

Closed nk9 closed 7 months ago

nk9 commented 7 months ago

Open Terminal and issue this command:

open itsycal://date/1969-07-20

With this PR, it will open the Itsycal menu item with the specified date selected. You can also open it to the current day by passing date/now.

nk9 commented 7 months ago

Note that, on macOS 14, the old hard-coded path to the D&T pref pane is no longer working. So this piece of the PR is more than a nice-to-have, it fixes an actual regression.

sfsam commented 7 months ago

Thanks for the PR. For some reason, when I give it a date, it opens the day prior. open itsycal://date/2023-01-01 will open December 31, 2022. open Itsycal://date/now works. Is this what you are seeing?

sfsam commented 7 months ago

FWIW, if I just use a normal NSDateFormatter it seems to work:

NSDateFormatter *format = [NSDateFormatter new];
format.dateFormat = @"yyyy-MM-dd";
nk9 commented 7 months ago

How strange! I wonder if it's something to do with time zones? Anyway, happy to use the plain Jane NSDateFormatter instead. I'll amend the PR.

sfsam commented 7 months ago

Yes, I think it might be time zones. On another note, I noticed you used NSAppKitVersionNumber to check API availability. I've been using the @available specifier for this purpose. I don't really know what the difference is. Do you? If there's no difference, could you use @available for consistency?

nk9 commented 7 months ago

I think @available is just newer, and Swift-y. I actually didn't know about it somehow! It's shorter and more ergonomic anyway, I've changed that too now.

sfsam commented 7 months ago

Thanks, it is working on my end now. I realize this is kind of overkill, but I think we can avoid an unnecessary NSDateFormatter allocation by checking the now case first. Sort of like...

if ("now") {
    dateURLReceived:now
} else {
    format = NSDateFormatter...
    date = dateFromString
    if (date) {
        dateURLReceived:date
    }
}

Please correct me if the above pseudocode misses something that the original is doing. I just think we can avoid a relatively expensive allocation.

sfsam commented 7 months ago

Thank you, looks great. I'd like to break this PR up into two since you've:

  1. Fixed a bug I didn't realize I had (open Date/Time...)!
  2. Added brand new functionality.

Can you do that? Then I will merge both PRs. There is some guidance in this gist in case it's helpful: https://gist.github.com/loilo/930f141d9acf89e9e734ffa042acd750

Finally, can you remove the .gitignore commit? I have that stuff in my global .gitignore since it applies to all Mac programming and I'd like to keep these PRs focussed.

sfsam commented 7 months ago

It looks like 1f121dd breaks the old behavior as the NSWorkspace call was removed?

nk9 commented 7 months ago

Quite right, sorry about that! I've returned that function to exactly what it was before.