Add C-c C-e key binding to save an org-ql-find session

[oantolin]

Since buffers-files is only present in a lexically scoped variable, I define the save command internally to org-ql-completing-read.

[alphapapa]

Thanks, this seems like a good idea. I'll merge it for v0.8 (I'd like to tag v0.7.2 first).

[oantolin]

Thanks, this seems like a good idea.

Well, it was your idea, you said you wanted something like helm-org-ql-save for org-ql-find. :)

[alphapapa]

Right, and for some reason I expected it to be contained within Embark, but of course, that's not feasible, so this makes sense. :)

[alphapapa]

This is great, I just need to:

• Update changelog
• Update usage/bindings

[alphapapa]

@oantolin Thank you very much for your work on this!

[oantolin]

Thanks for merging! I'm not sure how much I like the idea of remapping embark-collect as well, since the point of that command is that it always produces a buffer in embark-collect-mode. But it's a minor detail we can change later.

[oantolin]

So I tried embark-collect from org-ql-find (and from org-ql-find-in-agenda too, in case it somehow failed for multiple files) and everything seemed to work just fine. What problem did you run into?

I don't think there is any real problem with keeping the remap in org-ql-completing-read.el since people (like me!) who don't want it can easily undo it in their own configuration, but all the same I'd (weakly) recommend removing it.

Also, the way you did the remappping is much nicer than what I did for embark-export, maybe it would be better to do that for embark-export too (remap it to org-ql-completing-read-export) —if that works, which I haven't tested. Finally, keymap-set does have a syntax for remapping: "<remap> <embark-collect>", which I don't know how I found out about it since it isn't mentioned in the docstring for key-valid-p.

[alphapapa]

So I tried embark-collect from org-ql-find (and from org-ql-find-in-agenda too, in case it somehow failed for multiple files) and everything seemed to work just fine. What problem did you run into?

A buffer displayed with the entries, but activating them signaled an error.

Besides that, that buffer doesn't allow modifying or saving the search, so I'm not sure what reason there would be to offer it in addition to the full-featured view. Am I missing something?

Also, the way you did the remappping is much nicer than what I did for embark-export, maybe it would be better to do that for embark-export too (remap it to org-ql-completing-read-export) —if that works, which I haven't tested. Finally, keymap-set does have a syntax for remapping: "<remap> <embark-collect>", which I don't know how I found out about it since it isn't mentioned in the docstring for key-valid-p.

Hm, I'll take a look. Thanks.

[alphapapa]

I tried to do something like you suggested, but it doesn't work, and I don't know why. For some reason, the Embark commands end up calling the original definition of org-ql-completing-read-export rather than the one rebound with cl-letf, so it just gives an error. But the binding of C-c C-e does work, calling the rebound function. This is very surprising to me, as I would expect the keymap to point to the symbol, not to its function slot, so rebinding the function slot with cl-letf at any time ought to work.

Anyway, here's the patch. Let me know if you have any thoughts. Thanks.

diff --git a/org-ql-completing-read.el b/org-ql-completing-read.el
index 4db51e6..2e1f049 100644
--- a/org-ql-completing-read.el
+++ b/org-ql-completing-read.el
@@ -30,11 +30,9 @@ (require 'org-ql)
 
 (defvar-keymap org-ql-completing-read-map
   :doc "Active during `org-ql-completing-read' sessions."
-  "C-c C-e" #'org-ql-completing-read-export)
-
-;; `embark-collect' doesn't work for `org-ql-completing-read', so remap
-;; it to `embark-export' (which `keymap-set', et al doesn't allow).
-(define-key org-ql-completing-read-map [remap embark-collect] 'embark-export)
+  "C-c C-e" #'org-ql-completing-read-export
+  "<remap> <embark-collect>" #'org-ql-completing-read-export
+  "<remap> <embark-export>" #'org-ql-completing-read-export)
 
 ;;;; Customization
 
@@ -340,18 +338,16 @@ (cl-defun org-ql-completing-read
               (minibuffer-with-setup-hook
                   (lambda ()
                     (use-local-map (make-composed-keymap org-ql-completing-read-map (current-local-map))))
-                (cl-letf* (((symbol-function 'org-ql-completing-read-export)
-                            (lambda ()
-                              (interactive)
-                              (run-at-time 0 nil
-                                           #'org-ql-search
-                                           buffers-files
-                                           (minibuffer-contents-no-properties))
-                              (if (fboundp 'minibuffer-quit-recursive-edit)
-                                  (minibuffer-quit-recursive-edit)
-                                (abort-recursive-edit))))
-                           ((symbol-function 'embark-export)
-                            (symbol-function 'org-ql-completing-read-export)))
+                (cl-letf (((symbol-function 'org-ql-completing-read-export)
+                           (lambda ()
+                             (interactive)
+                             (run-at-time 0 nil
+                                          #'org-ql-search
+                                          buffers-files
+                                          (minibuffer-contents-no-properties))
+                             (if (fboundp 'minibuffer-quit-recursive-edit)
+                                 (minibuffer-quit-recursive-edit)
+                               (abort-recursive-edit)))))
                   (completing-read prompt #'collection nil t)))))
         ;; (debug-message "SELECTED:%S  KEYS:%S" selected (hash-table-keys table))
         (or (gethash selected table)

[oantolin]

A buffer displayed with the entries, but activating them signaled an error.

Weird. Here using embark-collect (after removing the remapping you added) does work: pressing RET on an entry takes you to the right place, and actions work too.

Besides that, that buffer doesn't allow modifying or saving the search, so I'm not sure what reason there would be to offer it in addition to the full-featured view. Am I missing something?

This also works correctly here: pressing g in the embark collect buffer reruns org-ql-find so you can modify the query if you want or call embark-collect, or embark-export or whatever. (This is what g normally does in both embark collect and embark export buffers.)

I wonder why that doesn't work on your end.

I tried to do something like you suggested, but it doesn't work, and I don't know why.

I tried too and it didn't work either, when I called embark-export through embark-act, but I seem to have gotten a different failure mode: I get an Org QL view buffer but with the heading as query instead of the minibuffer input. I don't fully understand how that happened but it's definitely because of embark-act's shenanigans. I'll look into it in a few days.

[alphapapa]

Thanks for checking into those things. I'll have to re-test them myself in case I did something wrong or had some temporary, buggy code defined somewhere.