notmuch-tag failing on trailing space

notmuch-tag failing on trailing space

[alan.schmitt]

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

Hello,

I have an issue when using the vertico completion system to add tags to
messages that boils down to the following: the notmuch-tag function is
called like this:
  notmuch-tag("id:CA+b3G33ad9PX5SuOwqaWS8TWBTtZcdguKtcQ3XmPvzSCsU..." ("+cwn "))
and it fails because of the extra space at the end.

Could notmuch-tag be modified to accept trailing spaces?

Thanks,

Alan

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

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

Re: notmuch-tag failing on trailing space

[Tomi Ollila]

On Tue, Jul 27 2021, alan schmitt wrote:

> Hello,
>
> I have an issue when using the vertico completion system to add tags to
> messages that boils down to the following: the notmuch-tag function is
> called like this:
>   notmuch-tag("id:CA+b3G33ad9PX5SuOwqaWS8TWBTtZcdguKtcQ3XmPvzSCsU..." ("+cwn "))
> and it fails because of the extra space at the end.
>
> Could notmuch-tag be modified to accept trailing spaces?

$ notmuch tag '+foo ' id:87im0w6u5r.fsf@m4x.org

works -- the tag is 'foo ' in this case.

The following fails (what I tried first based on your sample above):

$ notmuch tag id:87im0w6u5r.fsf@m4x.org '+foo '
Error: 'notmuch tag' requires at least one tag to add or remove.

(i.e. tags first then search terms)

>
> Thanks,
>
> Alan

Tomi

Re: notmuch-tag failing on trailing space

[Alan Schmitt]

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

Hello,

On 2021-07-27 10:24, alan.schmitt@polytechnique.org writes:

> I have an issue when using the vertico completion system to add tags to
> messages that boils down to the following: the notmuch-tag function is
> called like this:
>   notmuch-tag("id:CA+b3G33ad9PX5SuOwqaWS8TWBTtZcdguKtcQ3XmPvzSCsU..." ("+cwn "))
> and it fails because of the extra space at the end.
>
> Could notmuch-tag be modified to accept trailing spaces?

Someone on the doom discord suggested the following workaround.

#+begin_src emacs-lisp
  (defun trim-tag-changes (args)
    (list (car args) (mapcar #'s-trim-right (cadr args))))
  (advice-add 'notmuch-tag :filter-args #'trim-tag-changes)
#+end_src

Could it be integrated directly in the notmuch-tag function?

Best,

Alan

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

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

Re: notmuch-tag failing on trailing space

[David Bremner]

Alan Schmitt <alan.schmitt@polytechnique.org> writes:

>
> #+begin_src emacs-lisp
>   (defun trim-tag-changes (args)
>     (list (car args) (mapcar #'s-trim-right (cadr args))))
>   (advice-add 'notmuch-tag :filter-args #'trim-tag-changes)
> #+end_src
>
> Could it be integrated directly in the notmuch-tag function?
>

That particular function won't work for us because it would introduce a
new dependency on s.el. But if someone wants to make an equivalent using
only core emacs functions and integrate it into notmuch-tag, that seems
likely OK to me.

d

Re: notmuch-tag failing on trailing space

[David Edmondson]

On Monday, 2021-08-16 at 08:17:56 -07, David Bremner wrote:

> Alan Schmitt <alan.schmitt@polytechnique.org> writes:
>
>>
>> #+begin_src emacs-lisp
>>   (defun trim-tag-changes (args)
>>     (list (car args) (mapcar #'s-trim-right (cadr args))))
>>   (advice-add 'notmuch-tag :filter-args #'trim-tag-changes)
>> #+end_src
>>
>> Could it be integrated directly in the notmuch-tag function?
>>
>
> That particular function won't work for us because it would introduce a
> new dependency on s.el. But if someone wants to make an equivalent using
> only core emacs functions and integrate it into notmuch-tag, that seems
> likely OK to me.

What if someone has trailing spaces on their tags deliberately?

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.

Re: notmuch-tag failing on trailing space

[Alan Schmitt]

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

On 2021-08-16 08:17, David Bremner <david@tethera.net> writes:

> That particular function won't work for us because it would introduce a
> new dependency on s.el. But if someone wants to make an equivalent using
> only core emacs functions and integrate it into notmuch-tag, that seems
> likely OK to me.

What is the minimal emacs version notmuch requires? Because if it’s
above 24.4, we could use string-trim-right:

string-trim-right is a compiled function defined in subr-x.el.gz.

Signature
(string-trim-right STRING &optional REGEXP)

Documentation
Trim STRING of trailing string matching REGEXP.

REGEXP defaults to  "[ \\t\\n\\r]+".

Demos
:PROPERTIES:
:added:    24.4
:changes:  26.1 The optional REGEXP argument is added.
:END:

Best,

Alan

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

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

Re: notmuch-tag failing on trailing space

[Tomi Ollila]

On Mon, Aug 16 2021, David Edmondson wrote:

> On Monday, 2021-08-16 at 08:17:56 -07, David Bremner wrote:
>
>> Alan Schmitt <alan.schmitt@polytechnique.org> writes:
>>
>>>
>>> #+begin_src emacs-lisp
>>>   (defun trim-tag-changes (args)
>>>     (list (car args) (mapcar #'s-trim-right (cadr args))))
>>>   (advice-add 'notmuch-tag :filter-args #'trim-tag-changes)
>>> #+end_src
>>>
>>> Could it be integrated directly in the notmuch-tag function?
>>>
>>
>> That particular function won't work for us because it would introduce a
>> new dependency on s.el. But if someone wants to make an equivalent using
>> only core emacs functions and integrate it into notmuch-tag, that seems
>> likely OK to me.
>
> What if someone has trailing spaces on their tags deliberately?
>
> 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.

I'd also fix the source of the problem rather than the outcome...

Tomi

[PATCH] emacs: don't add space to tag completion candidates.

[David Bremner]

Apparently this messes up various third party completion
frameworks. This change does mean that users will have to hit space
after completing a tag change in order to enter another change.

As a bonus, remove the call to #'delete, since
completing-read-multiple already promises to remove empty strings.
---
 emacs/notmuch-tag.el | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index 536315e9..145f309f 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -429,17 +429,9 @@ initial input in the minibuffer."
 	    (set-keymap-parent map crm-local-completion-map)
 	    (define-key map " " 'self-insert-command)
 	    map)))
-    (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))))
+    (completing-read-multiple prompt tag-list
+			      nil nil initial-input
+			      'notmuch-read-tag-changes-history)))
 
 ;;; Tagging
 
-- 
2.33.0

Re: [PATCH] emacs: don't add space to tag completion candidates.

[inwit]

Works as expected under selectrum! Thanks!


On Sat Oct 23, 2021 at 7:05 PM CEST, David Bremner wrote:
> Apparently this messes up various third party completion
> frameworks. This change does mean that users will have to hit space
> after completing a tag change in order to enter another change.
>
> As a bonus, remove the call to #'delete, since
> completing-read-multiple already promises to remove empty strings.
> ---
> emacs/notmuch-tag.el | 14 +++-----------
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
> index 536315e9..145f309f 100644
> --- a/emacs/notmuch-tag.el
> +++ b/emacs/notmuch-tag.el
> @@ -429,17 +429,9 @@ initial input in the minibuffer."
> (set-keymap-parent map crm-local-completion-map)
> (define-key map " " 'self-insert-command)
> map)))
> - (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))))
> + (completing-read-multiple prompt tag-list
> + nil nil initial-input
> + 'notmuch-read-tag-changes-history)))
>  
> ;;; Tagging
>  
> --
> 2.33.0
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org

Re: [PATCH] emacs: don't add space to tag completion candidates.

[David Bremner]

"inwit" <inwit@sindominio.net> writes:

> Works as expected under selectrum! Thanks!
>
>

right, I guess the question is how annoying it is for users of standard
emacs completion. I generally enter one tag at a time, so it won't
really affect me.

d

Re: [PATCH] emacs: don't add space to tag completion candidates.

[Tomi Ollila]

On Sat, Oct 30 2021, David Bremner wrote:

> "inwit" <inwit@sindominio.net> writes:
>
>> Works as expected under selectrum! Thanks!
>>
>>
>
> right, I guess the question is how annoying it is for users of standard
> emacs completion. I generally enter one tag at a time, so it won't
> really affect me.

I recall doing adding many tags, but very seldom. I'd guess the need to
press space before entering next is tolerable (basing my guess on my
experiences w/ various experiences w/ filename completion...)


>
> d

Tomi

Re: [PATCH] emacs: don't add space to tag completion candidates.

[David Bremner]

David Bremner <david@tethera.net> writes:

> Apparently this messes up various third party completion
> frameworks. This change does mean that users will have to hit space
> after completing a tag change in order to enter another change.
>
> As a bonus, remove the call to #'delete, since
> completing-read-multiple already promises to remove empty strings.

Applied to master. I will let it sit there a while before release, but
if it ruins your life, please let me know.

d