emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: No Wayman <iarchivedmywholelife@gmail.com>
To: Allen Li <darkfeline@felesatra.moe>
Cc: emacs-orgmode@gnu.org
Subject: Re: [BUG] [BUG] inconsistent behavior when reading multiple tags [9.4.6 (9.4.6-g366444 @ /home/n/.emacs.d/straight/build/org/)]
Date: Fri, 03 Sep 2021 13:13:45 -0400	[thread overview]
Message-ID: <87ilzhjwkk.fsf@gmail.com> (raw)
In-Reply-To: <80pmtqp2wx.fsf@felesatra.moe>

[-- Attachment #1: Type: text/plain, Size: 2186 bytes --]


Allen Li <darkfeline@felesatra.moe> writes:

> Sorry about that (I wrote the crm patch).  I did not consider 
> people
> deliberately using invalid tag characters to separate tags. 
> It's an
> (un?)happy coincidence that org-set-tags-commands retains this 
> behavior,
> because the fast tags selection logic gets in the way.
>
> The inconsistency in behavior can be easily fixed by deleting 
> the code
> in org-set-tags-commands that replaces invalid tag characters 
> with ":".
>
> The question here is, which behavior do we want?  My philosphy 
> is that
> programs shouldn't try to silently re-interpret the user's 
> intentions.
> For example, if I accidentally mistyped the tag "green_blue" as
> "green-blue", I don't want Org to "helpfully" split one tag into 
> two
> tags "green:blue".  I may not realize the data corruption until 
> too
> late.

Consider the current behavior of `org-capture-fill-template':

If you were to mistype your tag as "green-blue" it would be 
captured as part of the headline string instead of a set of tags.
Future tag completions would not include any reference to it, and 
so you likely wouldn't notice until long after the fact 
(especially in the case of a template with a non-nil 
:immediate-finish).
So the risk of data corruption exists now by allowing the function 
to return an invalid tag string.
 
> There's also the option to only allow ":" and "," as separators.
> Whichever behavior we choose, I don't think it's worth making it 
> customizable.

I would rather not have to type ":" to separate Org tags when CRM 
allows me to use ","
(or another character of my choice) everywhere else. This violates 
the principle of least surprise.

An even simpler solution than my original patch is to have a 
defcustom which allows the user to customize
the CRM separator character(s) Org uses to read tags.

See the attached patch for a first draft implementation.

It defaults to what you've proposed (allowing ":" and "," to 
delimit tags).
It avoids introducing a new regexp variable which needs to be 
maintained in lockstep with `org-tag-re'.
It's customizable.
It informs the user about the tag delimiting characters in the 
prompt.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: org-tags-crm-separators-defcustom --]
[-- Type: text/x-patch, Size: 3339 bytes --]

From 34d292a60bc45923589646499b6e06c5cb3ca036 Mon Sep 17 00:00:00 2001
From: Nicholas Vollmer <iarchivedmywholelife@gmail.com>
Date: Fri, 3 Sep 2021 14:50:48 -0400
Subject: [PATCH] Add org-tags-crm-separators

commit message/formatting pending discussion of solution.
---
 lisp/org-capture.el |  7 +++++--
 lisp/org.el         | 17 ++++++++++++++---
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index c51744680..e9cd8f490 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -1740,9 +1740,12 @@ The template may still contain \"%?\" for cursor positioning."
 			    (org-add-colon-after-tag-completion t)
 			    (ins (mapconcat
 				  #'identity
-				  (let ((crm-separator "[ \t]*:[ \t]*"))
+				  (let* ((separators (or org-tags-crm-separators '(?: ?,)))
+                                         (crm-separator (org-tags-crm-regexp separators)))
                                     (completing-read-multiple
-				     (if prompt (concat prompt ": ") "Tags: ")
+				     (if prompt (concat prompt ": ")
+                                       (format "Tags (%s to delimit): "
+                                               (string-join (mapcar #'char-to-string separators) " ")))
 				     org-last-tags-completion-table nil nil nil
 				     'org-tags-history))
 				  ":")))
diff --git a/lisp/org.el b/lisp/org.el
index ce68f4692..d6739f7ed 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -2980,6 +2980,11 @@ is better to limit inheritance to certain tags using the variables
 	  (const :tag "Reverse alphabetical" org-string-collate-greaterp)
 	  (function :tag "Custom function" nil)))
 
+(defcustom org-tags-crm-separators '(?: ?,)
+  "List of characters used to separate tags during Org commands which read mutiple tags."
+  :type 'list
+  :group 'org-tags)
+
 (defvar org-tags-history nil
   "History of minibuffer reads for tags.")
 (defvar org-last-tags-completion-table nil
@@ -12007,6 +12012,10 @@ tags."
 	;; it now points to BLANK-START.  Use COLUMN instead.
 	(if in-blank? (org-move-to-column column) (goto-char origin))))))
 
+(defun org-tags-crm-regexp (chars)
+  "Return `crm-separator' regexp using CHARS as separators."
+  (format "[ \t]*%s[ \t]*" (regexp-opt (mapcar #'char-to-string chars))))
+
 (defun org-set-tags-command (&optional arg)
   "Set the tags for the current visible entry.
 
@@ -12065,11 +12074,13 @@ in Lisp code use `org-set-tags' instead."
 		      inherited-tags
 		      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]*"))
+		   (let* ((org-add-colon-after-tag-completion (< 1 (length table)))
+                          (separators (or org-tags-crm-separators '(?: ?,)))
+                          (crm-separator (org-tags-crm-regexp separators)))
 		     (mapconcat #'identity
                                 (completing-read-multiple
-			         "Tags: "
+                                 (format "Tags (%s to delimit): "
+                                         (string-join (mapcar #'char-to-string separators) " "))
 			         org-last-tags-completion-table
 			         nil nil (org-make-tag-string current-tags)
 			         'org-tags-history)
-- 
2.33.0


  parent reply	other threads:[~2021-09-03 19:26 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-26 17:04 [BUG] [BUG] inconsistent behavior when reading multiple tags [9.4.6 (9.4.6-g366444 @ /home/n/.emacs.d/straight/build/org/)] No Wayman
2021-08-31 10:17 ` Timothy
2021-09-03  2:05   ` No Wayman
2021-09-03  7:22     ` Timothy
2021-09-03 17:00       ` No Wayman
2021-09-03  6:51 ` Allen Li
2021-09-03  8:08   ` Timothy
2021-09-03 19:20     ` No Wayman
2021-09-17 17:09     ` No Wayman
2021-09-03 17:13   ` No Wayman [this message]
2021-09-03 19:42     ` No Wayman
2021-09-03 23:37       ` No Wayman
2021-09-27  8:35         ` Bastien
2021-09-27 15:19           ` No Wayman
2021-09-27 15:49             ` Bastien
2021-09-27 15:56               ` No Wayman
2021-09-06  0:48     ` Allen Li
2021-09-06 23:23       ` No Wayman
2021-09-12  3:37         ` Allen Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ilzhjwkk.fsf@gmail.com \
    --to=iarchivedmywholelife@gmail.com \
    --cc=darkfeline@felesatra.moe \
    --cc=emacs-orgmode@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).