[bug] notmuch-read-tag-changes, incompatability with consult, non-standard usage of completing-read-multiple

[bug] notmuch-read-tag-changes, incompatability with consult, non-standard usage of completing-read-multiple

[Natnael Kahssay]

[-- Attachment #1.1: Type: text/plain, Size: 2559 bytes --]



notmuch-read-tag-changes throws error, "Tag must be in the form of +'this_tag' or '-that_tag'" when used with consult. notmuch-read-tag-changes expects completing-read-multiple to strip crm-separator out of results. Consult's implementation of competing-read-multiple doesn't strip results.
I discussed with the author of the package here. https://github.com/minad/consult/issues/410

He states notmuch is using a non-standard and possibly tentative property of completing-read-multiple. I agree.

Below I have a version of notmuch-read-tag-changes that removes suffix crm-separator of returned candidates explicitly.
#+begin_src emacs-lisp
(defun notmuch-read-tag-changes (current-tags &optional prompt initial-input)
  "Prompt for tag changes in the minibuffer.

CURRENT-TAGS is a list of tags that are present on the message
or messages to be changed.  These are offered as tag removal
completions.  CURRENT-TAGS may contain duplicates.  PROMPT, if
non-nil, is the query string to present in the minibuffer.  It
defaults to \"Tags\".  INITIAL-INPUT, if non-nil, will be the
initial input in the minibuffer."
  (let* ((all-tag-list (notmuch-tag-completions))
     (add-tag-list (mapcar (apply-partially 'concat "+") all-tag-list))
     (remove-tag-list (mapcar (apply-partially 'concat "-") current-tags))
     (tag-list (append add-tag-list remove-tag-list))
     (prompt (concat (or prompt "Tags") " (+add -drop): "))
     (crm-separator " ")
     ;; By default, space is bound to "complete word" function.
     ;; Re-bind it to insert a space instead.  Note that <tab>
     ;; still does the completion.
     (crm-local-completion-map
      (let ((map (make-sparse-keymap)))
        (set-keymap-parent map crm-local-completion-map)
        (define-key map " " 'self-insert-command)
        map)))
    (mapcar (lambda (entry)
              (substring entry 0 (- (length crm-separator))))
            (delete "" (completing-read-multiple
                prompt
                ;; Append the separator to each completion so when the
                ;; user completes a tag they can immediately begin
                ;; entering another.  `completing-read-multiple'
                ;; ultimately splits the input on crm-separator, so we
                ;; don't need to strip this back off (we just need to
                ;; delete "empty" entries caused by trailing spaces).
                (mapcar (lambda (tag-op) (concat tag-op crm-separator)) tag-list)
                nil nil initial-input
		        'notmuch-read-tag-changes-history)))))
#+end_src

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [bug] notmuch-read-tag-changes, incompatability with consult, non-standard usage of completing-read-multiple

[David Bremner]

Natnael Kahssay <thisnkk@gmail.com> writes:

> notmuch-read-tag-changes throws error, "Tag must be in the form of +'this_tag' or '-that_tag'" when used with consult. notmuch-read-tag-changes expects completing-read-multiple to strip crm-separator out of results. Consult's implementation of competing-read-multiple doesn't strip results.
> I discussed with the author of the package here. https://github.com/minad/consult/issues/410
>
> He states notmuch is using a non-standard and possibly tentative property of completing-read-multiple. I agree.


This seems related to the discussion starting at

     https://nmbug.notmuchmail.org/nmweb/show/87im0w6u5r.fsf%40m4x.org

See in particular

    https://nmbug.notmuchmail.org/nmweb/show/m2pmud2xxa.fsf%40dme.org

Re: [bug] notmuch-read-tag-changes, incompatability with consult, non-standard usage of completing-read-multiple

[Natnael Kahssay]

[-- Attachment #1.1: Type: text/plain, Size: 3468 bytes --]

David Bremner <david@tethera.net> writes:
> This seems related to the discussion starting at
>
>      https://nmbug.notmuchmail.org/nmweb/show/87im0w6u5r.fsf%40m4x.org
>
> See in particular
>
>     https://nmbug.notmuchmail.org/nmweb/show/m2pmud2xxa.fsf%40dme.org

yes, it is the same problem.

I don't know how to reply to https://nmbug.notmuchmail.org/nmweb/show/m2pmud2xxa.fsf%40dme.org so I will reply here.

> What if someone has trailing spaces on their tags deliberately?

From my experiment below, notmuch, through completing-read-multiple, already trims crm-separator out of tags both left and right side regardless. Therefore, =string-trim-right= or even =string-trim= can be used. It is in package subr. Therefore notmuch wouldn't need new dependencies.

#+begin_src emacs-lisp :results org
(let ((crm-separator " "))
  (completing-read-multiple
   "> "
   (mapcar (lambda (tag-op) (concat tag-op crm-separator)) '("        first     " "second" "third"))))
#+end_src

#+RESULTS:
#+begin_src org
(first third second first)
#+end_src

> This seems like an oddity of the code in `notmuch-read-tag-changes' that
> appends a space to every possible completion. It would probably be a
> more annoying change to some users, but I'd be more inclined to remove
> that code than this change.

Given what I wrote above, (mapcar #'string-trim-right res) seems like a pleasant compromise. Users keep this convenience feature and the bug is solved without introducing new libraries and minimal modification.

#+begin_src emacs-lisp
(defun notmuch-read-tag-changes (current-tags &optional prompt initial-input)
  "Prompt for tag changes in the minibuffer.

CURRENT-TAGS is a list of tags that are present on the message
or messages to be changed.  These are offered as tag removal
completions.  CURRENT-TAGS may contain duplicates.  PROMPT, if
non-nil, is the query string to present in the minibuffer.  It
defaults to \"Tags\".  INITIAL-INPUT, if non-nil, will be the
initial input in the minibuffer."
  (let* ((all-tag-list (notmuch-tag-completions))
     (add-tag-list (mapcar (apply-partially 'concat "+") all-tag-list))
     (remove-tag-list (mapcar (apply-partially 'concat "-") current-tags))
     (tag-list (append add-tag-list remove-tag-list))
     (prompt (concat (or prompt "Tags") " (+add -drop): "))
     (crm-separator " ")
     ;; By default, space is bound to "complete word" function.
     ;; Re-bind it to insert a space instead.  Note that <tab>
     ;; still does the completion.
     (crm-local-completion-map
      (let ((map (make-sparse-keymap)))
        (set-keymap-parent map crm-local-completion-map)
        (define-key map " " 'self-insert-command)
        map)))
    (mapcar #'string-trim-right
            (delete "" (completing-read-multiple
                prompt
                ;; Append the separator to each completion so when the
                ;; user completes a tag they can immediately begin
                ;; entering another.  `completing-read-multiple'
                ;; ultimately splits the input on crm-separator, so we
                ;; don't need to strip this back off (we just need to
                ;; delete "empty" entries caused by trailing spaces).
                (mapcar (lambda (tag-op) (concat tag-op crm-separator)) tag-list)
                nil nil initial-input
                'notmuch-read-tag-changes-history)))))
#+end_src


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]