* [PATCH 1/1] notmuch-mutt: Fix tagging issues
@ 2013-09-06 22:15 Kevin J. McCarthy
2013-09-07 8:27 ` Tomi Ollila
0 siblings, 1 reply; 6+ messages in thread
From: Kevin J. McCarthy @ 2013-09-06 22:15 UTC (permalink / raw)
To: notmuch; +Cc: Stefano Zacchiroli
[-- Attachment #1: Type: text/plain, Size: 1247 bytes --]
This patch fixes three issues with "notmuch-mutt tag":
1. The message-id was not shell quoted.
Thanks to Jason Miller for the bug report and patch.
2. The tags passed into tag_action() were not being properly quoted.
The "join before shell_quote" was combining multiple tags into a
single argument to notmuch tag: '+one -two -three' instead of
'+one' '-two' '-three'. Note that shell_quote() will join array
arguments with a space after quoting each, so a join afterwards was
not needed.
3. A "--" was added between the tags and search-term as shown in the
current notmuch-tag man page.
---
contrib/notmuch-mutt/notmuch-mutt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/contrib/notmuch-mutt/notmuch-mutt b/contrib/notmuch-mutt/notmuch-mutt
index c69b35c..e5d6848 100755
--- a/contrib/notmuch-mutt/notmuch-mutt
+++ b/contrib/notmuch-mutt/notmuch-mutt
@@ -114,8 +114,9 @@ sub tag_action(@) {
defined $mid or die "notmuch-mutt: cannot find Message-Id, abort.\n";
system("notmuch tag "
- . shell_quote(join(' ', @_))
- . " id:$mid");
+ . shell_quote(@_)
+ . " -- "
+ . shell_quote("id:$mid"));
}
sub die_usage() {
--
1.8.4.rc3
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] notmuch-mutt: Fix tagging issues
2013-09-06 22:15 [PATCH 1/1] notmuch-mutt: Fix tagging issues Kevin J. McCarthy
@ 2013-09-07 8:27 ` Tomi Ollila
2013-09-07 15:57 ` Kevin J. McCarthy
0 siblings, 1 reply; 6+ messages in thread
From: Tomi Ollila @ 2013-09-07 8:27 UTC (permalink / raw)
To: Kevin J. McCarthy, notmuch; +Cc: Stefano Zacchiroli
On Sat, Sep 07 2013, "Kevin J. McCarthy" <kevin@8t8.us> wrote:
> This patch fixes three issues with "notmuch-mutt tag":
>
> 1. The message-id was not shell quoted.
> Thanks to Jason Miller for the bug report and patch.
>
> 2. The tags passed into tag_action() were not being properly quoted.
> The "join before shell_quote" was combining multiple tags into a
> single argument to notmuch tag: '+one -two -three' instead of
> '+one' '-two' '-three'. Note that shell_quote() will join array
> arguments with a space after quoting each, so a join afterwards was
> not needed.
>
> 3. A "--" was added between the tags and search-term as shown in the
> current notmuch-tag man page.
The patch looks good to me, just that I cannot test it. +1.
If it weren't inconsistent what is there now a simpler alternative could be:
system qw/notmuch tag/, @_, '--', "id:$mid";
or even
exec qw/notmuch tag/, @_, '--', "id:$mid";
... as these bypasses the intermediate shell altogether.
Tomi
> ---
> contrib/notmuch-mutt/notmuch-mutt | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/notmuch-mutt/notmuch-mutt b/contrib/notmuch-mutt/notmuch-mutt
> index c69b35c..e5d6848 100755
> --- a/contrib/notmuch-mutt/notmuch-mutt
> +++ b/contrib/notmuch-mutt/notmuch-mutt
> @@ -114,8 +114,9 @@ sub tag_action(@) {
> defined $mid or die "notmuch-mutt: cannot find Message-Id, abort.\n";
>
> system("notmuch tag "
> - . shell_quote(join(' ', @_))
> - . " id:$mid");
> + . shell_quote(@_)
> + . " -- "
> + . shell_quote("id:$mid"));
> }
>
> sub die_usage() {
> --
> 1.8.4.rc3
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
--
"kaik on mänt!"
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] notmuch-mutt: Fix tagging issues
2013-09-07 8:27 ` Tomi Ollila
@ 2013-09-07 15:57 ` Kevin J. McCarthy
2013-09-07 17:07 ` [PATCH v2 " Kevin J. McCarthy
0 siblings, 1 reply; 6+ messages in thread
From: Kevin J. McCarthy @ 2013-09-07 15:57 UTC (permalink / raw)
To: Tomi Ollila, notmuch; +Cc: Stefano Zacchiroli
[-- Attachment #1: Type: text/plain, Size: 636 bytes --]
Tomi Ollila wrote:
> The patch looks good to me, just that I cannot test it. +1.
>
> If it weren't inconsistent what is there now a simpler alternative could be:
>
> system qw/notmuch tag/, @_, '--', "id:$mid";
>
> or even
>
> exec qw/notmuch tag/, @_, '--', "id:$mid";
>
> ... as these bypasses the intermediate shell altogether.
Thank you for the great suggestion. I completely forgot about the list
form of system while focusing on the patch :-). Since we don't need the
shell in this case, it makes sense to use that form.
Please hold off on committing this patch. I will post a revised patch.
-Kevin
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/1] notmuch-mutt: Fix tagging issues
2013-09-07 15:57 ` Kevin J. McCarthy
@ 2013-09-07 17:07 ` Kevin J. McCarthy
2013-09-08 15:50 ` Tomi Ollila
2013-09-09 1:52 ` David Bremner
0 siblings, 2 replies; 6+ messages in thread
From: Kevin J. McCarthy @ 2013-09-07 17:07 UTC (permalink / raw)
To: notmuch; +Cc: Tomi Ollila, Stefano Zacchiroli
[-- Attachment #1: Type: text/plain, Size: 1249 bytes --]
This patch fixes three issues with "notmuch-mutt tag":
1. The message_id was not shell quoted.
Thanks to Jason Miller for the bug report and patch.
2. The tags passed into tag_action() were not being properly quoted.
The "join before shell_quote" was combining multiple tags into a
single argument to notmuch tag: '+one -two -three' instead of
'+one' '-two' '-three'.
3. A "--" was added between the tags and search-term as shown in the
current notmuch-tag man page.
Thanks to Tomi Ollila for suggesting the simple fix of using
the list form of system(), which bypasses the shell.
---
contrib/notmuch-mutt/notmuch-mutt | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/contrib/notmuch-mutt/notmuch-mutt b/contrib/notmuch-mutt/notmuch-mutt
index c69b35c..4969e4b 100755
--- a/contrib/notmuch-mutt/notmuch-mutt
+++ b/contrib/notmuch-mutt/notmuch-mutt
@@ -113,9 +113,7 @@ sub tag_action(@) {
my $mid = get_message_id();
defined $mid or die "notmuch-mutt: cannot find Message-Id, abort.\n";
- system("notmuch tag "
- . shell_quote(join(' ', @_))
- . " id:$mid");
+ system("notmuch", "tag", @_, "--", "id:$mid");
}
sub die_usage() {
--
1.8.4.rc3
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] notmuch-mutt: Fix tagging issues
2013-09-07 17:07 ` [PATCH v2 " Kevin J. McCarthy
@ 2013-09-08 15:50 ` Tomi Ollila
2013-09-09 1:52 ` David Bremner
1 sibling, 0 replies; 6+ messages in thread
From: Tomi Ollila @ 2013-09-08 15:50 UTC (permalink / raw)
To: Kevin J. McCarthy, notmuch; +Cc: Stefano Zacchiroli
On Sat, Sep 07 2013, "Kevin J. McCarthy" <kevin@8t8.us> wrote:
> This patch fixes three issues with "notmuch-mutt tag":
>
> 1. The message_id was not shell quoted.
> Thanks to Jason Miller for the bug report and patch.
>
> 2. The tags passed into tag_action() were not being properly quoted.
> The "join before shell_quote" was combining multiple tags into a
> single argument to notmuch tag: '+one -two -three' instead of
> '+one' '-two' '-three'.
>
> 3. A "--" was added between the tags and search-term as shown in the
> current notmuch-tag man page.
>
> Thanks to Tomi Ollila for suggesting the simple fix of using
> the list form of system(), which bypasses the shell.
> ---
LGTM.
Tomi
> contrib/notmuch-mutt/notmuch-mutt | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/contrib/notmuch-mutt/notmuch-mutt b/contrib/notmuch-mutt/notmuch-mutt
> index c69b35c..4969e4b 100755
> --- a/contrib/notmuch-mutt/notmuch-mutt
> +++ b/contrib/notmuch-mutt/notmuch-mutt
> @@ -113,9 +113,7 @@ sub tag_action(@) {
> my $mid = get_message_id();
> defined $mid or die "notmuch-mutt: cannot find Message-Id, abort.\n";
>
> - system("notmuch tag "
> - . shell_quote(join(' ', @_))
> - . " id:$mid");
> + system("notmuch", "tag", @_, "--", "id:$mid");
> }
>
> sub die_usage() {
> --
> 1.8.4.rc3
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
--
"kaik on mänt!"
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] notmuch-mutt: Fix tagging issues
2013-09-07 17:07 ` [PATCH v2 " Kevin J. McCarthy
2013-09-08 15:50 ` Tomi Ollila
@ 2013-09-09 1:52 ` David Bremner
1 sibling, 0 replies; 6+ messages in thread
From: David Bremner @ 2013-09-09 1:52 UTC (permalink / raw)
To: Kevin J. McCarthy, notmuch; +Cc: Tomi Ollila, Stefano Zacchiroli
"Kevin J. McCarthy" <kevin@8t8.us> writes:
> This patch fixes three issues with "notmuch-mutt tag":
>
> 1. The message_id was not shell quoted.
> Thanks to Jason Miller for the bug report and patch.
>
> 2. The tags passed into tag_action() were not being properly quoted.
> The "join before shell_quote" was combining multiple tags into a
> single argument to notmuch tag: '+one -two -three' instead of
> '+one' '-two' '-three'.
>
> 3. A "--" was added between the tags and search-term as shown in the
> current notmuch-tag man page.
pushed.
d
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-09-09 1:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-06 22:15 [PATCH 1/1] notmuch-mutt: Fix tagging issues Kevin J. McCarthy
2013-09-07 8:27 ` Tomi Ollila
2013-09-07 15:57 ` Kevin J. McCarthy
2013-09-07 17:07 ` [PATCH v2 " Kevin J. McCarthy
2013-09-08 15:50 ` Tomi Ollila
2013-09-09 1:52 ` David Bremner
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).