unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* notmuch-tag failing on trailing space
@ 2021-07-27  8:24 alan.schmitt
  2021-07-27  9:37 ` Tomi Ollila
  2021-08-16  9:03 ` Alan Schmitt
  0 siblings, 2 replies; 12+ messages in thread
From: alan.schmitt @ 2021-07-27  8:24 UTC (permalink / raw)
  To: notmuch


[-- 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 --]



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: notmuch-tag failing on trailing space
  2021-07-27  8:24 notmuch-tag failing on trailing space alan.schmitt
@ 2021-07-27  9:37 ` Tomi Ollila
  2021-08-16  9:03 ` Alan Schmitt
  1 sibling, 0 replies; 12+ messages in thread
From: Tomi Ollila @ 2021-07-27  9:37 UTC (permalink / raw)
  To: alan.schmitt, notmuch

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: notmuch-tag failing on trailing space
  2021-07-27  8:24 notmuch-tag failing on trailing space alan.schmitt
  2021-07-27  9:37 ` Tomi Ollila
@ 2021-08-16  9:03 ` Alan Schmitt
  2021-08-16 15:17   ` David Bremner
  1 sibling, 1 reply; 12+ messages in thread
From: Alan Schmitt @ 2021-08-16  9:03 UTC (permalink / raw)
  To: notmuch


[-- 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 --]



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: notmuch-tag failing on trailing space
  2021-08-16  9:03 ` Alan Schmitt
@ 2021-08-16 15:17   ` David Bremner
  2021-08-16 15:48     ` David Edmondson
  2021-08-16 15:52     ` notmuch-tag failing on trailing space Alan Schmitt
  0 siblings, 2 replies; 12+ messages in thread
From: David Bremner @ 2021-08-16 15:17 UTC (permalink / raw)
  To: Alan Schmitt, notmuch

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: notmuch-tag failing on trailing space
  2021-08-16 15:17   ` David Bremner
@ 2021-08-16 15:48     ` David Edmondson
  2021-08-17  5:52       ` Tomi Ollila
  2021-08-16 15:52     ` notmuch-tag failing on trailing space Alan Schmitt
  1 sibling, 1 reply; 12+ messages in thread
From: David Edmondson @ 2021-08-16 15:48 UTC (permalink / raw)
  To: David Bremner, Alan Schmitt, notmuch

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: notmuch-tag failing on trailing space
  2021-08-16 15:17   ` David Bremner
  2021-08-16 15:48     ` David Edmondson
@ 2021-08-16 15:52     ` Alan Schmitt
  1 sibling, 0 replies; 12+ messages in thread
From: Alan Schmitt @ 2021-08-16 15:52 UTC (permalink / raw)
  To: David Bremner, notmuch


[-- 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 --]



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: notmuch-tag failing on trailing space
  2021-08-16 15:48     ` David Edmondson
@ 2021-08-17  5:52       ` Tomi Ollila
  2021-10-23 17:05         ` [PATCH] emacs: don't add space to tag completion candidates David Bremner
  0 siblings, 1 reply; 12+ messages in thread
From: Tomi Ollila @ 2021-08-17  5:52 UTC (permalink / raw)
  To: David Edmondson, David Bremner, Alan Schmitt, notmuch

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] emacs: don't add space to tag completion candidates.
  2021-08-17  5:52       ` Tomi Ollila
@ 2021-10-23 17:05         ` David Bremner
  2021-10-29 15:30           ` inwit
  2021-12-04 16:59           ` David Bremner
  0 siblings, 2 replies; 12+ messages in thread
From: David Bremner @ 2021-10-23 17:05 UTC (permalink / raw)
  To: Tomi Ollila, David Edmondson, David Bremner, Alan Schmitt,
	notmuch

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] emacs: don't add space to tag completion candidates.
  2021-10-23 17:05         ` [PATCH] emacs: don't add space to tag completion candidates David Bremner
@ 2021-10-29 15:30           ` inwit
  2021-10-30  9:31             ` David Bremner
  2021-12-04 16:59           ` David Bremner
  1 sibling, 1 reply; 12+ messages in thread
From: inwit @ 2021-10-29 15:30 UTC (permalink / raw)
  To: David Bremner, Tomi Ollila, David Edmondson, Alan Schmitt,
	notmuch

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] emacs: don't add space to tag completion candidates.
  2021-10-29 15:30           ` inwit
@ 2021-10-30  9:31             ` David Bremner
  2021-10-31 19:34               ` Tomi Ollila
  0 siblings, 1 reply; 12+ messages in thread
From: David Bremner @ 2021-10-30  9:31 UTC (permalink / raw)
  To: inwit, Tomi Ollila, David Edmondson, Alan Schmitt, notmuch

"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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] emacs: don't add space to tag completion candidates.
  2021-10-30  9:31             ` David Bremner
@ 2021-10-31 19:34               ` Tomi Ollila
  0 siblings, 0 replies; 12+ messages in thread
From: Tomi Ollila @ 2021-10-31 19:34 UTC (permalink / raw)
  To: David Bremner, inwit, David Edmondson, Alan Schmitt, notmuch

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] emacs: don't add space to tag completion candidates.
  2021-10-23 17:05         ` [PATCH] emacs: don't add space to tag completion candidates David Bremner
  2021-10-29 15:30           ` inwit
@ 2021-12-04 16:59           ` David Bremner
  1 sibling, 0 replies; 12+ messages in thread
From: David Bremner @ 2021-12-04 16:59 UTC (permalink / raw)
  To: Tomi Ollila, David Edmondson, Alan Schmitt, notmuch

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-12-04 17:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-27  8:24 notmuch-tag failing on trailing space alan.schmitt
2021-07-27  9:37 ` Tomi Ollila
2021-08-16  9:03 ` Alan Schmitt
2021-08-16 15:17   ` David Bremner
2021-08-16 15:48     ` David Edmondson
2021-08-17  5:52       ` Tomi Ollila
2021-10-23 17:05         ` [PATCH] emacs: don't add space to tag completion candidates David Bremner
2021-10-29 15:30           ` inwit
2021-10-30  9:31             ` David Bremner
2021-10-31 19:34               ` Tomi Ollila
2021-12-04 16:59           ` David Bremner
2021-08-16 15:52     ` notmuch-tag failing on trailing space Alan Schmitt

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.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).