Re: [PATCH 1/1] notmuch-mutt: Fix tagging issues

Subject: Re: [PATCH 1/1] notmuch-mutt: Fix tagging issues

Date: Sat, 07 Sep 2013 11:27:55 +0300

To: Kevin J. McCarthy, notmuch@notmuchmail.org

Cc: Stefano Zacchiroli

From: Tomi Ollila


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!"

Thread: