Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misbehaving when combining multiple link predicates #279

Closed
telenieko opened this issue May 20, 2022 · 6 comments
Closed

Misbehaving when combining multiple link predicates #279

telenieko opened this issue May 20, 2022 · 6 comments
Assignees
Labels

Comments

@telenieko
Copy link

Hi,

First of all, Thank You Adam for such an amazing package. I'm just scratching the surface of it but I already see some interesting workflows that org-ql can enable. Not only that but by reading the source code I've learnt a lot (I'm very new to Emacs, so lots to learn).

Awesome!

I noticed some strange behaviour when combining link with or. I've tried to reduce it to the simplest example to reproduce (it was an org-babel file hence the current-buffer; but Github does not allow for attaching of .org files, neither understands what babel is):

* TODO Item 1
+ [[notmuch:id:one-message-id][One Message ID]]
+ [[notmuch:id:two-message-id][Two Message ID]]

* TODO Item 2
:PROPERTIES:
:EMAIL:    [[notmuch:id:three-message-id][Three Message ID]]
:END:
+ [[notmuch:id:one-message-id][One Message ID]]

With the sample file above I try several things:

;; Expect Items 1 & 2. Does return items 1 & 2
(org-ql-select (current-buffer)
  '(link "notmuch:id:one-message-id")
  :action '(org-get-heading t t))

;; Expect Items 1 & 2. Does return items 1 & 2
(org-ql-select (current-buffer)
  '(link :target "notmuch:id:one-message-id")
  :action '(org-get-heading t t))

So the link predicate works as expected. But strange things happen when you want to combine multiple link predicates with either and or or:

;; Expect Item 1, returns nothing
(org-ql-select (current-buffer)
  '(and (link "notmuch:id:one-message-id")
        (link "notmuch:id:two-message-id"))
  :action '(org-get-heading t t))

;; Expect Item 1, returns nothing
(org-ql-select (current-buffer)
  '(and (link :target "notmuch:id:one-message-id")
        (link :target "notmuch:id:two-message-id"))
  :action '(org-get-heading t t))

And with or:

;; Expect Items 1 & 2, returns Item 1
(org-ql-select (current-buffer)
  '(or (link "notmuch:id:one-message-id")
       (link "notmuch:id:three-message-id"))
  :action '(org-get-heading t t))

;; Expect Items 1 & 2, returns Item 1
(org-ql-select (current-buffer)
  '(or (link :target "notmuch:id:one-message-id")
       (link :target "notmuch:id:three-message-id"))
  :action '(org-get-heading t t))

So... I'm missing something; But according to my org-mode I've already been at it for 2 hours without success!

Some more background. I use e-mail a lot, so much that more than half my org-mode TODO items link to e-mails. So I wondered: Why not have a command that finds all TODO items that link to them? aka, backlinks from TODO to emails.

I use notmuch, so I think it is very easy: just combine notmuch-show-get-messages-ids with a query (or (link :target "notmuch:id:....") (link...)). And done. Except for the above. Now I realise (reading the code of org-sidebar) that this is probably a very underperforming way to do it, so I'll give it a go using regular expressions. Will be fun too :)

Nonetheless I'm really curious to understand why the above examples did not work.

I am on org-ql 0.6.1

@alphapapa alphapapa self-assigned this May 27, 2022
@alphapapa alphapapa added the bug label May 27, 2022
@alphapapa
Copy link
Owner

alphapapa commented May 27, 2022

Hi Marc,

Thanks for the kind words. I'm glad this tool is useful to you.

And thanks for this very well-written bug report. Turns out it was a simple oversight in the code. That should fix it.

Regarding performance: yes, due to the current implementation of the link predicate, using it in or'ed queries will not be optimized to a regexp search, and it will be slower. Instead you could do something like this:

(org-ql-search BUFFER
  `(link :target ,(rx-to-string `(seq bos "notmuch:id:" (or "one" "two"))) :regexp-p t))

That is, construct the regexp yourself and call the link predicate with it, being sure to tell it that it's a regexp.

@telenieko
Copy link
Author

telenieko commented May 27, 2022

Hi,

And thanks for this very well-written bug report. Turns out it was a simple oversight in the code. That should fix it.

Thank you !!!

Regarding performance: yes, due to the current implementation of the link predicate, using it in or'ed queries will not be optimized to a regexp search, and it will be slower. Instead you could do something like this:

(org-ql-search BUFFER
  `(link :target ,(rx-to-string `(seq bos "notmuch:id:" (or "one" "two"))) :regexp-p t))

Cool. For anyone coming here in the future (ie: searching how to do backlinks from org to notmuch), I paste below my current working code. Anyone coming here in the future: please do not use this issue as a support forum! :)

The org-backlinks predicate is generic so it can be used not only for notmuch but for any links:

;; Searching backlinks
;; consider this a pluralized version of the link predicate
(org-ql-defpred org-backlinks (&rest targets)
  "Search for entries that have any of those links."
  :normalizers
  ((`(,predicate-names . ,targets)
    `(link :target ,(rx-to-string `(or ,@targets))
           :regexp-p t)
    )))

(defun mf-email--get-links-to-current-thread()
  "Find todo items linking to this notmuch email thread."
  (interactive)
  (let* ((message-ids (notmuch-show-get-messages-ids))
         (targets (cl-loop for tgt in message-ids
                           collect (concat "notmuch:" tgt)))
         (query `(org-backlinks ,@targets)))
    (org-ql-search (org-agenda-files) query)))

(define-key notmuch-show-mode-map (kbd "C-z t b") 'mf-email--get-links-to-current-thread)

@alphapapa
Copy link
Owner

Cool! I'm impressed at how quickly you learned how to use the defpred macro and the pcase-based normalizers. You don't seem like someone new to Elisp. :)

FYI, you don't need that cl-loop in the normalizer, because it's just collecting the list's items into another list. You can just use ,@targets.

As well, you might want to adjust how you're building the regexp: as it is now, you seem to be duplicating the notmuch: part, and I don't think regexp-opt (which is eventually called by calling rx-to-string, AFAIK) will optimize that out. (Although it's unlikely to make a noticeable difference, I guess.)

@telenieko
Copy link
Author

Cool! I'm impressed at how quickly you learned how to use the defpred macro and the pcase-based normalizers. You don't seem like someone new to Elisp. :)

Thank you! Lots, lots of trial and error got me there. Also the tutorial helped a lot (and web search!).

FYI, you don't need that cl-loop in the normalizer, because it's just collecting the list's items into another list. You can just use ,@targets.

Updated the code above! thx.

As well, you might want to adjust how you're building the regexp: as it is now, you seem to be duplicating the notmuch: part, and I don't think regexp-opt (which is eventually called by calling rx-to-string, AFAIK) will optimize that out. (Although it's unlikely to make a noticeable difference, I guess.)

It's not the most pretty regexp but doesn't look too bad either (it's regex after all!) and the rx-to-string kind of goes a long way optimizing (I was surprised by this):

\(?:notmuch:id:\(?:87fsl0e9eh\.fsf@domain\.xyz\|\(?:first-i\|id-secon\)d@mail\.gmail\.com\)\)
\(?:notmuch:id:\(?:87fsl0e9eh\.fsf@domain\.xyz\|\(?:first-id\|id-made-up\)@mail\.gmail\.com\)\)

@alphapapa
Copy link
Owner

@telenieko Glad to hear that the tutorial helped. You're one of only a few people who have read it carefully and used it to write their own predicate. Do you have any feedback on the tutorial? Was it easy to understand? Were any parts hard to follow or unhelpful?

@telenieko
Copy link
Author

telenieko commented May 28, 2022

(...) Do you have any feedback on the tutorial? Was it easy to understand? Were any parts hard to follow or unhelpful?

I'll collect my thoughts and file an issue with feedback, hopefully tomorrow!

Edit, feedback posted on: #280

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants