org-ql icon indicating copy to clipboard operation
org-ql copied to clipboard

Creating a new heading during a org-ql-completing-read session

Open akirak opened this issue 3 years ago • 7 comments

Hello,

I want to get an olp entered from the user in org-ql-completing-read. Specifically, I am thinking of the following use cases:

  • Let the user choose a heading to clock into. If the heading does not exist, fire org-capture with the input as the title.
  • Let the user choose a heading to refile an entry into. If the heading does not exist, create a new heading.

This can be solved by adding a history variable for completing-read in the command. If the function returns nil, the user can get the latest item from the history and split the string with / to get an olp. This is hacky, but it achieves the desired effect with minimal changes to the implementation.

An alternative way is to change

(gethash selected table)

to

(or (gethash selected table)
            (when return-olp
              (split-string selected "/")))

and add :return-olp argument to the function. The extra argument is needed to avoid confusion in normal use cases, but it is more complicated (in API and implementation) than adding the history variable.

What is the best way to solve the problem?

akirak avatar Aug 29 '22 06:08 akirak

Hi Akira,

Yes, clocking and refiling are two obvious uses for org-ql-completing-read. (Ideally Org would have a framework that would allow the function used for selecting entries for various purposes to be set by the user, and we could have a mode that would use org-ql-completing-read for that.)

Anyway, you can see the org-ql-refile command I already implemented here: https://github.com/alphapapa/org-ql/blob/e41cdb45912f1731b5b060f9e05d011096663655/org-ql-find.el#L88 (That file may not be the permanent home for that command.)

As for getting an outline path, the function returns a marker, so it should be as simple as this:

(org-with-point-at (org-ql-completing-read ...)
  (org-get-outline-path))

So there should be no need to change the API.

alphapapa avatar Aug 29 '22 09:08 alphapapa

Thank you for your quick response.

What I want is a unified completion API that can handle both existing entries (returning a marker) and non-existent ones (returning an olp).

I have the following function that can create a heading from an olp (even if its parent does not exist), which can be integrated with org-capture and org-refile:

(defun akirak-org-goto-or-create-olp (olp)
  (let ((level 1))
    (dolist (heading olp)
      (unless (catch 'found
                (while (re-search-forward (format org-complex-heading-regexp-format
                                                  (regexp-quote heading))
                                          nil t)
                  (when (equal level (- (match-end 1) (match-beginning 1)))
                    (throw 'found t))))
        (org-end-of-subtree)
        (insert "\n" (make-string level ?*) " " heading))
      (org-narrow-to-subtree)
      (setq level (org-get-valid-level (1+ level))))))

It would be nice if I could build the olp using org-ql-completing-read.

akirak avatar Aug 29 '22 09:08 akirak

What I want is a unified completion API that can handle both existing entries (returning a marker) and non-existent ones (returning an olp).

That is a nice idea, but returning a non-existent heading would seem to violate the concern of org-ql-completing-read, whose purpose is to return matches for a search query (not to return non-matches). If the user selects a heading with org-ql-completing-read, how is the function to know whether the user wants that heading, or a new, non-existent one to be made under it? As well, the input given to org-ql-completing-read is not an outline path but tokens passed (by default) to the rifle predicate; it wouldn't make sense for a new outline path to be generated from that input.

I think you will need to do something like having an alternative key binding, like M-RET, that would use the heading selected with org-ql-completing-read and prompt the user to insert a new child heading under it.

alphapapa avatar Aug 29 '22 09:08 alphapapa

That is a nice idea, but returning a non-existent heading would seem to violate the concern of org-ql-completing-read

I understand your point.

you will need to do something like having an alternative key binding

It doesn't look easy to define such a keybinding. It would be nice if org-ql-completing-read used a dedicated keymap!

akirak avatar Aug 29 '22 09:08 akirak

Ok, feel free to propose a patch that adds one. :)

alphapapa avatar Aug 29 '22 09:08 alphapapa

It would be nice if org-ql-completing-read used a dedicated keymap!

I tried to implement this idea, but it turns out that this is not easy, at least for vertico users. Vertico uses its own keymap:

(defun vertico--setup ()
  "Setup completion UI."
  (setq vertico--input t
        vertico--candidates-ov (make-overlay (point-max) (point-max) nil t t)
        vertico--count-ov (make-overlay (point-min) (point-min) nil t t))
  ;; Set priority for compatibility with `minibuffer-depth-indicate-mode'
  (overlay-put vertico--count-ov 'priority 1)
  (setq-local completion-auto-help nil
              completion-show-inline-help nil)
  (use-local-map vertico-map)
  (add-hook 'post-command-hook #'vertico--exhibit nil 'local))

Even if I could somehow create a heading during the minibuffer session, the completion table would never include a marker to the new entry, which would mean org-ql-completing-read returns nil for non-existent entries. The problem may not be easy to solve, so I will change the title of this ticket and leave it open.

akirak avatar Aug 29 '22 11:08 akirak

Ok, maybe we can figure something out in the future. Thanks.

alphapapa avatar Sep 02 '22 04:09 alphapapa

Not sure if I understand this issue correctly, but if you want a local keymap for a command you can use minibuffer-with-setup-hook and create a new local map, composed with the existing minibuffer map. See for example jinx--correct-setup from my Jinx package:

  • https://github.com/minad/jinx/blob/9982192145500bb9fa6f2d7d2ed4db6cd7f905d9/jinx.el#L706
  • https://github.com/minad/jinx/blob/9982192145500bb9fa6f2d7d2ed4db6cd7f905d9/jinx.el#L630

This approach should be compatible with all completion UIs (at least as long as you don't override some important bindings which are already in use).

minad avatar May 25 '23 18:05 minad

@minad Thanks.

Since this issue was originally filed, embark has become quite a powerful tool, and it now has support for various Org-related actions: https://github.com/oantolin/embark/issues/562 So I think the best way to support this would be for org-ql to support embark, so we would gain all of its power.

alphapapa avatar May 25 '23 20:05 alphapapa

Repurposing this issue for Embark support.

@oantolin made this comment:

Ha! I just added a text property storing the marker to my local copy of org-ql! I didn't do it to fix #350, but to enable embark actions on headings listed by org-ql-find. If this gets merged, and the org-ql-completing-read completion table gets a category metadatum, then I can easily write a tiny org-ql-embark package offering support for embark-collect and for using Org heading actions on org-ql-find candidates.

Omar, if you're still interested in working on that, please let me know how I can help. One feature I'd like to add is the equivalent of helm-org-ql-save, which in Embark terms would be like "exporting".

alphapapa avatar Sep 07 '23 00:09 alphapapa

I am interested in working on Embark support.

For actions on the org-ql-find completion candidates I think the just-merged org-marker text properties are enough. With that I can have Embark jump to the heading before executing an action.

For export, can I just stick the strings that have the org-marker property into a buffer and put it in org-agenda mode? I'm not very familiar with org internals.

Oh, I just took a peek at my org agenda and boy do those lines have text properties! So maybe it's not as simple as inserting the org-q-find candidates into a buffer. Does org-ql have a helper function that can make an org-ql view buffer from a list of strings with org-marker text properties? Probably not.

oantolin avatar Sep 07 '23 02:09 oantolin

I am interested in working on Embark support.

Hurray! :)

For export, can I just stick the strings that have the org-marker property into a buffer and put it in org-agenda mode? I'm not very familiar with org internals.

No, the model to follow is here: https://github.com/alphapapa/org-ql/blob/f65b1d91a49468e176bd733d3b930ec4b7ed4cf1/helm-org-ql.el#L162-L168 Basically, we want to take the search query string and pass it to org-ql-search; it will then show the same results in an org-ql-view buffer (similar to org-agenda).

alphapapa avatar Sep 07 '23 03:09 alphapapa

Oh, that doesn't quite fit the Embark export model, which is supposed to be a function of the list of completion candidates, not of the minibuffer input used to produce them. But I guess we can cheat.

oantolin avatar Sep 07 '23 04:09 oantolin

The cheating solution would be slightly limited compared to usual embark export: normally you can use embark-select to choose some subset of the completion candidates and export only those.

oantolin avatar Sep 07 '23 04:09 oantolin

Mmh. Is there an analogue of the helm-org-ql-buffers-files dynamic variable for org-ql-find? I think that information is locked away in a lexically scoped function parameter, right?

oantolin avatar Sep 07 '23 04:09 oantolin

@oantolin The function helm-org-ql-save seems more like a normal command which can be bound in the minibuffer-local-map and not something necessarily related to Embark? This could be implemented via minibuffer-setup-hook without Embark. But maybe I am missing the point. @alphapapa Btw, thanks for merging #357 and fixing #350!

minad avatar Sep 07 '23 04:09 minad

Yes, definitely, @minad. That kind of function wouldn't really take any advantage of Embark infrastructure. It would be better bound to some key in the minibuffer via a setup hook as you suggest. But then it should be just made a part of org-ql-find (which would make the problem with the buffer-files being lexically scoped ---if I'm right about that--- not a problem anymore).

oantolin avatar Sep 07 '23 04:09 oantolin

Embark users, if they wished, could remap embark-export to that org-ql-save function while in org-ql-find, just to take advantage of muscle memory.

oantolin avatar Sep 07 '23 04:09 oantolin

It could still be that some org-ql-specific support is in order, leading to an embark-org-ql package, or a tiny bit of integration code in a (with-eval-after-load 'embark) block in org-ql? I suspect that we may need something similar like embark-consult-goto-grep.

minad avatar Sep 07 '23 04:09 minad

Oh, yes, if we want embark-collect to work, definitely a default action override is needed. There's enough to warrant an embark-org-ql package even if this export-like functionality is more properly left inside org-ql:

  • To be able to target org-ql-find candidates we need either to change org-ql-completing-read to add a completion category, or, it can be solved externally in an embark-org-ql package via a transformer that refines general targets that have an org-marker text property to some org-ql-result type.
  • The embark-org-heading-map should be registered as the action map for the org-ql-result type.
  • A pre-action hook that uses the org-marker text propery to jump to the org heading before running the action would be needed.
  • For RET to work in embark-collect buffers a default action override is needed similar to the embark-consult-goto-grep function you mentioned.

oantolin avatar Sep 07 '23 04:09 oantolin

(Clearly I forgot the first line of that comment by the time I wrote the last one: damn Chrome and its tiny-by-default text areas. 😄 It's actually pretty unusual that I wrote that in Chrome instead of copy-pasting from Emacs.)

oantolin avatar Sep 07 '23 04:09 oantolin

Yes, sounds good. I suggest to add a completion category here. I consider this good style because of completion-category-overrides, Embark, Marginalia, etc., and such that no guessing and transforming is needed.

minad avatar Sep 07 '23 04:09 minad

I suggest to add a completion category here. I consider this good style

Oh, I agree, and proposed that a little over two months ago.

oantolin avatar Sep 07 '23 04:09 oantolin

I just remembered there was a discussion on oantolin/embark#639 about how to best support acting on agenda items and you, @minad, pointed out that the current support for the org-agenda commands is pretty superfluous since those commands already have bindings in org-agenda-mode-map. I then suggested a different approach exactly like what is needed here: instead of using the agenda-item commands as actions, to support using the org heading actions via a pre-action hook that jumps to the heading. If I did that in embark-org, all that would be needed here for actions is just adding the completion category and they would work out of the box!

oantolin avatar Sep 07 '23 04:09 oantolin

I'm inclined to make that change in embark-org and make a one-line PR here adding the completion category and no embark-org-ql package would be needed. Oh, and then someone would write the org-ql-save command to add here...

oantolin avatar Sep 07 '23 04:09 oantolin

Woah! org-ql-completing-read overrides completion-styles! That seems crazy. Why not just provide a completion table? Overriding completion-styles totally messes with my recursive minibuffer habit.

oantolin avatar Sep 07 '23 06:09 oantolin

OK. If anyone wants to try preliminary Embark support, just use the remote-org-heading branch of Embark and use this Marginalia configuration:

(add-to-list 'marginalia-command-categories '(org-ql-find . org-remote-heading))

Actions offered are the same as for the org heading at point in an org buffer, and when acting on an org-ql-find candidate you visibly jump to the actual heading before the action occurs. (I also implemented a version that jumps invisibly but feel that might be a little confusing; that version uses embark-org--at-remote-heading as an around action hook instead of using embark-org-goto-remote-heading as a pre-action hook).

You can currently run embark-collect (whose usual binding is shadowed 🙄) or embark-export to get the same result: a collect buffer that looks a little oddly formatted but works.

oantolin avatar Sep 07 '23 06:09 oantolin

On 9/7/23 08:14, Omar Antolín Camarena wrote:

Woah! |org-ql-completing-read| /overrides/ |completion-styles|! That seems crazy. Why not just provide a completion table? Overring |completion-styles| totally messes with my recursive minibuffer habit.

Org-ql does that such that the entire input is passed to the completion table. Furthermore the input is interpreted in its own way, different from the usual completion style filtering. The approach is similar to consult-grep, where the prefix part of #prefix#suffix is not used for filtering.

minad avatar Sep 07 '23 07:09 minad

I realized that a little later, @MINAD. Even if you can get a completion table to produce the right candidates you definitely don't want to match against them in the normal way. I posted here in shock that my recursive minibuffer was broken without pausing to think. The correct thing to say was instead that setq-local should be used, not let.

oantolin avatar Sep 07 '23 11:09 oantolin

Of the original use cases @akirak mentioned, the current Embark support takes care of clocking in. For refiling a special action function should be wrriten, because if you use org-refile on heading X it means refile X somewhere else, and I think @akirak wanted an action to refile the heading that was at point before running org-ql-find to X. That extra function would be just like org-ql-refile except non-interactive and it would take a string with an org-marker text property instead of directly taking the marker.

oantolin avatar Sep 08 '23 12:09 oantolin