spegoraro / org-alert

System notifications of org agenda items
GNU General Public License v3.0
277 stars 25 forks source link

Use org-ql to match org headings to be alerted. #41

Open rickyson96 opened 2 weeks ago

rickyson96 commented 2 weeks ago

Hi! I've implemented a way to fetch only the related headings for alerting.

I kind of cheat a little using org-ql, but I think it worked out pretty well.

It should fix #27, though I'm not sure if I have handled all use cases.

I try to implement it in a way that's backward compatible, so some customization might not be in place right now.

We could also potentially make the org-alert-check customizable too, but I've put this off for now since I think the current implementation is good enough.

To use this new feature, we need to add:

;; or setopt for fancier option :)
(setq org-alert-match-string 'org-ql
      ;; use non-greedy regex to match the `from' time.
      org-alert-time-match-string "\\(?:SCHEDULED\\|DEADLINE\\):.*?<.*?\\([0-9]\\{2\\}:[0-9]\\{2\\}\\).*>")
ntBre commented 2 weeks ago

Wow thanks for this! I didn't know about org-ql. I need to take a slightly closer look at how it works, but it seems really promising. Is there any reason not to use org-ql exclusively? (ah besides backwards compatibility) If we're already adding the dependency, I wouldn't mind making this the default/only behavior. org-alert-match-string has always felt very brittle to me anyway.

rickyson96 commented 2 weeks ago

If you're okay with the breaking changes, I think we can remove much of the old logic and depend solely on org-ql (note the changes on run-at-time too because it enables the cronjob behaviour).

rickyson96 commented 2 weeks ago

I've updated the implementation to only use org-ql.

The problem is that I've borked the REMINDERN property. I've done a little bit of testing to see if we could do it with org-ql, but haven't get to implement it on the PR yet. :D

ntBre commented 2 weeks ago

Thanks again for doing this! I've added a few superficial comments, mostly trying to avoid changes unrelated to adding org-ql. I probably won't get to a fuller review until this weekend, but besides these minor issues it looks very promising.

I'm not necessarily against these other changes (except changing the default after-event behavior), but I would rather separate them from the main task at hand to make this easier to review.