Remember to cover the basics, that is, what you expected to happen and what in fact did happen. You don't know how to make a good report? See

    https://orgmode.org/manual/Feedback.html#Feedback

Your bug report will be posted to the Org mailing list.
------------------------------------------------------------------------

622f9fa76c8ee0766b15945c013b0950d703b955 ("org: Use crm for completing tags") lexically binds crm-separator to ":" while prompting for multiple tags in
org-set-tags-command and org-capture-fill-template.

org-set-tags-command correctly parses the tags when using a comma (the default crm-separator) to separate them, but completion is broken unless ":" is used as
the separator.
org-capture-fill-template incorrectly parses this.

Reproducible from emacs -Q:

org-set-tags-command:
1. Insert a new Org entry in the a buffer 2. M-x org-set-tags-command 3. At the prompt enter "one,two,three" 4. Entry's tags are correctly set to :one:two:three:
org-capture-fill-template:
1. evaluate (org-capture-fill-template "%^g")
2. At the prompt enter "one,two,three"
3. String returned is incorrect: ":one,two,three:"


I can think of a couple ways we could improve this and make the behavior consistent. We could introduce a defcustom which allows the user to choose whether or not Org will rebind crm-separator during commands/functions which utilize
completing-read-multiple.
e.g.

#+begin_src emacs-lisp :lexical t
(defcustom org-contextual-crm-separator t
 "Use contextual binding of crm-separator during Org commands.
For example, when setting tags, \":\" is used as the separator.
If nil, `crm-separator' is used.")
#+end_src

Alternatively, we could change the lexical binding we're using during these commands/functions. Instead of binding the separator explicitly to "[ \t]*:[ \t]*", we could instead use a regexp which will match any character not described by org-tag-re. The benefit of this is that completing-read-multiple completion will work for any separator that doesn't match org-tag-re transparently and the results should be consistent between org-set-tags-command and org-capture-fill-template.

The attached patch implements that change, which seems to do the right thing from my testing. I can polish it, but I wanted to get opinions on the solution first.

Note this does not change the case of org-todo-list, which binds
crm-separator to "|". I chose not to touch that because I'm not fully aware of what the restrictions on characters for todo-keywords are, if any. org-todo-list also does the right thing by mentioning the separator in the prompt.

We could also combine the approaches:

- Introduce a defcustom that determines if we're going to contextually rebind crm-separator
- Use a broader regexp when contextually binding crm-separator
- Mention the separator in the prompt when rebound

Thoughts?


Emacs : GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.30, cairo version 1.17.4)
of 2021-08-02
Package: Org mode version 9.4.6 (9.4.6-g366444 @ /home/n/.emacs.d/straight/build/org/)

>From 2ec28b530eb28cf788c38556ec3cb97f3bacd4af Mon Sep 17 00:00:00 2001
From: Nicholas Vollmer <iarchivedmywholel...@gmail.com>
Date: Thu, 26 Aug 2021 12:51:27 -0400
Subject: [PATCH] RFC: broader regexp for lexically bound crm-separator

For testing purposes. Ammended commit message pending.
---
 lisp/org-capture.el | 2 +-
 lisp/org.el         | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index c51744680..47b47c3ca 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -1740,7 +1740,7 @@ The template may still contain \"%?\" for cursor positioning."
 			    (org-add-colon-after-tag-completion t)
 			    (ins (mapconcat
 				  #'identity
-				  (let ((crm-separator "[ \t]*:[ \t]*"))
+				  (let ((crm-separator org-tag-crm-separator))
                                     (completing-read-multiple
 				     (if prompt (concat prompt ": ") "Tags: ")
 				     org-last-tags-completion-table nil nil nil
diff --git a/lisp/org.el b/lisp/org.el
index ce68f4692..f5e396cba 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -602,6 +602,9 @@ not contribute to the agenda listings.")
 (defconst org-tag-re "[[:alnum:]_@#%]+"
   "Regexp matching a single tag.")
 
+(defconst org-tag-crm-separator "[ \t]*[^[:alnum:]_@#%][ \t]*"
+  "Regexp used to determine `crm-separator' when reading multiple tags.")
+
 (defconst org-tag-group-re "[ \t]+\\(:\\([[:alnum:]_@#%:]+\\):\\)[ \t]*$"
   "Regexp matching the tag group at the end of a line, with leading spaces.
 Tags are stored in match group 1.  Match group 2 stores the tags
@@ -12066,7 +12069,7 @@ in Lisp code use `org-set-tags' instead."
 		      table
 		      (and org-fast-tag-selection-include-todo org-todo-key-alist))
 		   (let ((org-add-colon-after-tag-completion (< 1 (length table)))
-                         (crm-separator "[ \t]*:[ \t]*"))
+                         (crm-separator org-tag-crm-separator))
 		     (mapconcat #'identity
                                 (completing-read-multiple
 			         "Tags: "
-- 
2.33.0

Reply via email to