selectrum is not able to select multiple tags in Org mode

[pedrormjunior]

In Org mode, for selecting tags for a headline, selectrum seems to be not working. It works for selecting a single tag but not for multiple tags.

One particularity of tag selection in Org mode (compared to other autocomplete contexts) is that we should be able to autocomplete and select a tag, then press : and autocomplete to select another tag, and so on. If some tags are already present on a headline, when calling org-set-tags-command, the tags already present goes to the minibuffer and the new tag(s) would be concatenated to the ones already present.

The bug appears in two contexts: (1) When inserting tags for a headline with no previous tag, we can autocomplete and select the first one, but after inserting the tag separator (:), selectrum is not able to autocomplete the second one. (2) When modifying a headline that already contains tags, selectrum does not keep the current tags on the minibuffer and start the selection of a new tag from scratch, which means that the previous tags are lost.

[clemera]

Thanks for the detailed report! Could you provide a recipe for reproducing it (including your Emacs and Org version)? I have tested

#+tags: some tags to test

* Testing :<C-M-i>

and

#+tags: some tags to test

* Testing :some:tags:<C-M-i>

which seem to work correctly.

[pedrormjunior]

I have just double checked by opening Emacs with -Q argument. The problem, as described before, persists. I have some extra observations, though: (1) When trying to use complete-symbol function (i.e., <C-M-i>), the completion works just fine, however, the problem described before happens with the use of org-set-tags-command through <C-c C-q> or through C-c C-c with the cursor at the top of the target headline. You, @clemera, can reproduce the bug with the example you provided.

Another observation is that, (2) when trying to add the first tag ever (in a file that does not even contain the line #+tags: ... nor any other tag set to any headline), the following error echos on the echo area: Error in post-command-hook (selectrum--minibuffer-post-command-hook): (wrong-type-argument number-or-marker-p nil), indicating in the prompt of the minibuffer the following: 0 Tags: [default value: none]. It might or might not be related to the reported bug.

Tests with GNU Emacs 28.0.50, commit 9a7aab2d9e, and Org mode version 9.3.7, commit eac255d91.

I have installed selectrum yesterday, so I have yet no familiarity with the code, otherwise I could try to help on this. Thank you!

[clemera]

Thanks, sorry I somehow did not notice you mentioned org-set-tags-command in your first comment, I can reproduce it now. But I don't use tags in org-mode myself and I'm not sure how this commands is supposed to work: When I use default completion (without selectrum) it inserts the present tags initially but I get no completion for available tags after the first one. It seems to me that org-set-tags-command should better use completing-read-multiple.

The reason that the present tags are not initially inserted with Selectrum is because in selectrum-completing-read the initial-input argument is ignored as per @raxod502:

     ;; Don't pass `initial-input'. We use it internally but it's
    ;; deprecated in `completing-read' and doesn't work well with the
    ;; Selectrum paradigm except in specific cases that we control.

When we pass initial-input it works like with default completion. I still wonder why org-set-tags-command doesn't use completing-read-multiple which seems like a perfect fit here.

[clemera]

it inserts the present tags initially but I get no completion for available tags after the first one

After some searching I discovered this has been a known problem for some time now and helm also handles this specially. Here is a quick fix which should give you a nice UI with selectrum:

(defun org-set-tags-command-multiple (orig &optional arg)
  (cl-letf (((symbol-function #'completing-read)
             (lambda (prompt collection &optional predicate require-match initial-input
                             hist def inherit-input-method)
               (when initial-input
                 (setq initial-input
                       (replace-regexp-in-string
                        ":" ","
                        (replace-regexp-in-string
                         "\\`:" "" initial-input))))
               (let ((res (completing-read-multiple
                           prompt collection predicate require-match initial-input
                           hist def inherit-input-method)))
                 (mapconcat #'identity res ":")))))
    (let ((current-prefix-arg arg))
      (call-interactively orig))))

(advice-add #'org-set-tags-command :around #'org-set-tags-command-multiple)

When we would want to include this into selectrum we could also decide to keep the ":" as separator in the input after #140 got merged.

[raxod502]

Perhaps this could be contributed upstream to Org by someone sufficiently enterprising! Seems like it would fix the problem with Helm, too.

[clemera]

I have send a patch to the mailing list, will see how that goes.

[clemera]

Turns out usually you get completion after the first tag with default completion (org-tags-completion-function completion table handles this). It didn't work for me because some issue with orderless which I have set in completion-styles. Completion frameworks have trouble with org-set-tags-command because they usually filter according to current input afterwards and aren't aware of ignoring the already inserted tags for filtering.

Ideally I can convince org devs to switch to completing-read-multiple in the meantime we could use the idea of an alist for completion tables and also use it for the purpose of delegating to other completion handlers similar to helm-completing-read-handlers-alist.

[clemera]

After looking a bit more into it here are some more details:

The problems with this completion table are that it is dynamic (so (funcall collection "" predicate t) isn't enough, see #114) and that Selectrum does its own refinement with what it thinks should be the current input with selectrum-refine-candidates-function after getting those candidates. But in this case the completion table adjusts the current input before passing it to all-completions. So the completion table already filtered the candidates and does not expect any additional filtering afterwards. Another thing to note is that all-completions and try-completions use completion-regexp-list which can be set by completion-styles which Selectrum has currently no support for. So for this to be accurate Selectrum would need to support completion-styles so the matching style stays the same.

Another problem is selectrum-insert-current-candidate because it does not know what part of the minibuffer contents is considered the actual input. Maybe this could be fixed by calling the table with flag nil to indicate a try-completion operation and use the result of that for insertion.

Luckily there are not many cases where completion tables are written like that (but on the mailing list org-agenda-filter-completion-function was mentioned as another table which acts like this). It would be great if we could solve this on a more general level, it works with icomplete so it should be possible to some extend. But I'm afraid it would be a lot of work (and probably also means to give up on selectrum-refine-candidates-function in favour of completion-styles) so I think as an intermediate solution we should go with the completion table alist (but make it private so we can potentially remove it later without issues).

[raxod502]

Okay, your analysis sounds all good to me. I hope that it will be possible to preserve selectrum-refine-candidates-function when adding completion-styles support, by using the former unless the current completion style is something nonstandard.

[clemera]

I hope that it will be possible to preserve selectrum-refine-candidates-function when adding completion-styles support, by using the former unless the current completion style is something nonstandard.

Maybe we could make it so that selectrum-refine-candidates-function would default to nil and only when it is set it would be used otherwise filtering would happen according to completion-styles.

[raxod502]

That seems reasonable, my only concern would be preserving some reasonable behavior out of the box. (I'm not sure what Selectrum's handling of the default completion style would look like, and whether it would be as good as what we have currently.)

[clemera]

I'm not sure what Selectrum's handling of the default completion style would look like

You can test with:

(setq selectrum-refine-candidates-function
      (lambda (input cands)
        (nconc
         ;; this will make use of completion-styles internally
         (completion-all-completions input cands
                                     nil (length input))
         nil)))

(setq selectrum-highlight-candidates-function
      (lambda (str cands)
        cands))

The built-in completion style that would make the most sense with Selectrum would probably be flex for Emacs 27 and emacs22 for earlier versions. For flex I don't get highlighting currently but maybe that is because I tested with the backported helm-flex style (I'm still on Emacs 26). Many completion-styles make not much sense for Selectrum so I'm not sure if it makes sense to add support for them, too.

But as described above this issue won't be solved by gaining support for completion-styles. The main problem with the org completion table is that it is very tightly integrated with the way the default completion works (where you press TAB to recompute the candidates which will take the current input into account with this completion table).

[raxod502]

Okay, after playing around with it, I suppose that would be workable (as a default, considering that most people will be using prescient.el anyway, or an alternative like orderless).

[clemera]

Tag completion also doesn't work with orderless and also not the built-in substring style. Org should get fixed to use completing-read-multiple.

[minad]

Interesting. Is this an example of a broken dynamic completion table? It works in Vertico if I use (setq completion-styles '(basic orderless)). But in the end you still have to submit via vertico-exit-input (C-RET) instead of the usual RET. The UI is far from good and I agree with you that completing-read-multiple should be used for that. But even the broken table will probably work if you merge #532.

Did you send an emacs bug report?

[clemera]

Indeed with #532 it works using basic for example but adding multiple tags (using TAB) will erase the input line (also in vertico) while with default completion you can append more tags after the colon.

[clemera]

Did you send an emacs bug report?

I once even tried to fix this but gave up I have to search the org mailing list.

[clemera]

It ended here, at that time I wasn't aware that it is broken for substring with default completion as well.

[minad]

Indeed with #532 it works using basic for example but adding multiple tags (using TAB) will erase the input line (also in vertico) while with default completion you can append more tags after the colon.

Yes, this completion table does not use completion boundaries. Therefore TAB replaces the input completely. It only works with TAB completion minibuffer-complete since it handles the try-completion action specially. Basically this table assumes TAB completion and the basic completion style. It is incompatible with more "complicated" completion styles like substring or others since these completion styles do use the empty prefix.

(defun org-tags-completion-function (string _predicate &optional flag)
  "Complete tag STRING.
FLAG specifies the type of completion operation to perform.  This
function is passed as a collection function to `completing-read',
which see."
  (let ((completion-ignore-case nil)	;tags are case-sensitive
	(confirm (lambda (x) (stringp (car x))))
	(prefix ""))
    (when (string-match "^\\(.*[-+:&,|]\\)\\([^-+:&,|]*\\)$" string)
      (setq prefix (match-string 1 string))
      (setq string (match-string 2 string)))
    (pcase flag
      (`t (all-completions string org-last-tags-completion-table confirm))
      (`lambda (assoc string org-last-tags-completion-table)) ;exact match?
      (`nil
       (pcase (try-completion string org-last-tags-completion-table confirm)
	 ((and completion (pred stringp))
	  (concat prefix
		  completion
		  (if (and org-add-colon-after-tag-completion
			   (assoc completion org-last-tags-completion-table))
		      ":"
		    "")))
	 (completion completion)))
      (_ nil))))

I consider this a bit of a broken table since it tries to control everything. If it would use completion boundaries everything would just work. And this is just exactly what completing-read-multiple does. Is there a reason to not use completing-read-multiple for this in org? I mean the fix should be pretty easy. From what I see there is not much magic in there except for adding colons?

My recommendation regarding such tables is to bind M-TAB and M-RET to the default completion commands (https://github.com/minad/vertico/#tab-completion). Or do something like the following, similar in Selectrum. Icomplete also does not work differently I think given the org-tags-completion-function.

(defun fix-org-set-tags-command (orig &optional arg)
  (let ((vertico-map (make-composed-keymap nil vertico-map))
        (completion-styles '(basic)))
    (define-key vertico-map [remap vertico-insert] #'minibuffer-complete)
    (define-key vertico-map [remap vertico-exit] #'minibuffer-force-complete-and-exit)
    (funcall orig arg)))
(advice-add #'org-set-tags-command :around #'fix-org-set-tags-command)

[minad]

Another idea - if you have tables making these kind of arbitrary completions then the returned all-completions can be empty or at least must not necessarily match the minibuffer input with the active completion-styles. In this case any fancy UI like Selectrum/Vertico should deactivate itself and disallow selection with the arrow keys. Instead it should just use the minibuffer-complete commands. It may still show the current candidates as returned by the completion table.

The UI should switch from a selection mode to a pure completion mode, where nothing else than TAB completion is allowed.

[clemera]

I also think that the table is broken and it creates problems for other frameworks like helm, too.

Is there a reason to not use completing-read-multiple for this in org?

They would have been okay with switching to crm (see the linked mail) but it needed more work and I'm not a user of it myself so I wasn't motivated enough to proceed.

[minad]

They would have been okay with switching to crm (see the linked mail) but it needed more work and I'm not a user of it myself so I wasn't motivated enough to proceed.

Understandably. I recently started to use org more and more and like it. It does not fit my taste regarding minimalism - it is a pretty complex beast. org.el is 20k lines something, but this is also a good benchmark for consult-line 😆

But I am not sure if I would want to claim that you can implement the org functionality with much less code. I guess if you concentrate on the most important features then yes and that could fit me better. Maybe only outliner+markup+agenda. But then there is also babel which is nice and a bazillion of other features so no.

[minad]

Btw, the table is also broken with default completion and completion-cycle-threshold/=nil.

[clemera]

I only use org for basic things so far. I played a bit with org-roam and it seems nice but I'm waiting for v2 before I go all in ;)

[minad]

I only use org for basic things so far.

Tags are not basic?

I played a bit with org-roam and it seems nice but I'm waiting for v2 before I go all in ;)

org-roam is too much for me for now. It is certainly a nice concept but I think basic org already goes far.

Regarding the broken table my current approach is this, which should also work in Selectrum given #532. This gives you back the default completion as is together with the fancy minibuffer UI, basically just like Icomplete. Note that Icomplete also does not work to its full extent correctly with this table (rotating through the candidates does not work or I don't understand how it should work). But given this disable-selection I don't think the UI is that bad.

(defun disable-selection (orig &optional arg)
  (let ((vertico-map minibuffer-local-completion-map)
        (completion-cycle-threshold nil)
        (completion-styles '(basic)))
    (funcall orig arg)))
(advice-add #'org-set-tags-command :around #'disable-selection)

The problem is once again that it is not possible to detect if the table if of such a specific nature, that it expects these specific settings. Maybe I should suggest on emacs-devel to add a metadata flag 'assumes-basic-completion, 'assumes-tab-completion or something like this. Then the completion UI could switch from its selection mode to the basic tab completion mode as in the advice above. You may have seen the long discussion on emacs-devel about selecting-read/completing-read.

[clemera]

Tags are not basic?

Maybe they are but I don't have a use for tags personally, files with outline trees and links are enough for me personally. Org-roam is mainly interesting to me because of backlinks.

Regarding the broken table my current approach is this, which should also work in Selectrum given #532

Thanks, also note the advice delegating to crm I gave at the start of this issue here. I think I prefer that as the command is nicer to use this way.

The problem is once again that it is not possible to detect if the table if of such a specific nature, that it expects these specific settings. Maybe I should suggest on emacs-devel to add a metadata flag 'assumes-basic-completion, 'assumes-tab-completion or something like this. Then the completion UI could switch from its selection mode to the basic tab completion mode as in the advice above.

I would prefer if such tables would get fixed so they work with other UIs as well. I don't know how many such tables actually exists in Emacs, maybe these strange org tables are the only ones?

You may have seen the long discussion on emacs-devel about selecting-read/completing-read.

I have seen it and the parts I read were interesting, I hope some of the suggested improvements make it through.

[minad]

Org-roam is mainly interesting to me because of backlinks.

Yes that's a good feature.

I think I prefer that as the command is nicer to use this way.

Sure. My point is rather how these completion uis could work with such problematic tables in general which assume full control.

I would prefer if such tables would get fixed so they work with other UIs as well.

I agree with you to some extent at least. I think the status quo is not well defined.

But I somehow see a point in having tables which take full control. I mentioned the selecting-read/completing-read discussion. There was also the argument that maybe it would suffice to add some metadata to the table which signals if it assumes full control.

The difference is that our uis assume selection behavior. You can scroll between candidates and select. But assume you have some table which generates arbitrary text out of the input and completes/replaces the input with arbitrary text. Such table will only work with basic by construction.

But then one can also argue that when doing such full completions/text replacement you could still display a list of possible completions and select between those making the distinction meaningless. I tend to this opinion but have not chimed in too much in the mailing list regarding that since I was more concerned with the small completing-read proposals.

[andersjohansson]

Is this solved by Commit 622f9fa in org?

[minad]

@andersjohansson Yes!

[raxod502]

Awesome, sounds like we could close this then.

[raxod502]

This thread is being closed automatically by Tidier because it is labeled with "waiting on response" and has not seen any activity for 90 days. But don't worry—if you have any information that might advance the discussion, leave a comment and I will be happy to reopen the thread :)