dengste / org-caldav

Caldav sync for Emacs orgmode
GNU General Public License v3.0
712 stars 105 forks source link

Fix: Don't trigger error in org-id #288

Closed real-or-random closed 8 months ago

real-or-random commented 8 months ago

org-id-update-id-locations signals an error if org-id-track-globally is nil.

Moreover, this commit makes sure we call it with the SILENT parameter set t to suppress messages.

jackkamm commented 8 months ago

I am curious how well org-caldav will work if org-id-track-globally is nil. Have you tested it much?

For example, the first unit test fails if I do (setq org-id-track-globally nil), even on your branch, because org-id-update-id-locations is also triggered by org-id-find. But maybe it's because that unit test uses 2 org-mode files -- maybe it would work if only using 1 org-mode file? I include the backtrace at the bottom.

Also, I prefer SILENT to be nil, it's alerted me to some accidental duplicate IDs in the past. I would accept an option to set it though.

Here's the relevant part of the backtrace:

Backtrace for test ‘org-caldav-01-sync-test’:
  signal(error ("Please turn on ‘org-id-track-globally’ if you want..."))
  error("Please turn on `org-id-track-globally' if you want...")
  org-id-update-id-locations(nil t)
  org-id-find("orgcaldavtest@org1" t)
  ...
  org-caldav-generate-md5-for-org-entry("orgcaldavtest@org1")
  ...
  org-caldav-update-eventdb-from-org(#<buffer org-caldav-VK8Whi>)
  ...
real-or-random commented 8 months ago

I am curious how well org-caldav will work if org-id-track-globally is nil. Have you tested it much?

Okay, yeah, it has been working for me for a few days, but I only the sync cal->org and no complex cases, e.g., no changed events so far. I took a closer look at the code, and now I share your doubts. For example, org-caldav-update-events-in-org really seems to rely on org-id.

I guess then it's better to close this and instead document the requirement that org-id-track-globally is non-nil. It took me a while to figure out what's wrong after I had set this to nil... Not sure what's the best approach for this. We could just note it in the README, but it will probably also be good to have some check in the code. The error message "Please turn on `org-id-track-globally' if you want..." really wasn't helpful. (Thanks, but I don't want!)