From: Tomi Ollila <tomi.ollila@iki.fi>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH 1/2] tag: Disallow adding malformed tags to messages
In-Reply-To: <1351284853-24809-1-git-send-email-amdragon@mit.edu>
References: <1351284853-24809-1-git-send-email-amdragon@mit.edu>
User-Agent: Notmuch/0.14+59~gf9031cd (http://notmuchmail.org) Emacs/24.2.1
	(x86_64-unknown-linux-gnu)
Date: Sat, 27 Oct 2012 00:17:43 +0300
Message-ID: <m2haphue4o.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Fri, Oct 26 2012, Austin Clements wrote:

> This disallows adding empty tags, since nothing but confusion follows
> in their wake, and disallows adding tags that begin with "-" because
> they are also confusing, the tag "-" is impossible to remove using the
> CLI, and because the syntax for removing such tags conflicts with long
> argument syntax.
>
> This does not place any restrictions on what tags can be removed, as
> that would make it difficult for people who have the misfortune of
> already having malformed tags to remove these tags.
> ---

LGTM (NEWS too)

Tomi


>  notmuch-tag.c |   11 +++++++++++
>  test/tagging  |    4 ++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 7d18639..d15f1ed 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -203,6 +203,17 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>  	    break;
>  	}
>  	if (argv[i][0] == '+' || argv[i][0] == '-') {
> +	    if (argv[i][0] == '+' && argv[i][1] == '\0') {
> +		fprintf(stderr, "Error: tag names cannot be empty.\n");
> +		return 1;
> +	    }
> +	    if (argv[i][0] == '+' && argv[i][1] == '-') {
> +		/* This disallows adding the non-removable tag "-" and
> +		 * enables notmuch tag to take long options in the
> +		 * future. */
> +		fprintf(stderr, "Error: tag names must not start with '-'.\n");
> +		return 1;
> +	    }
>  	    tag_ops[tag_ops_count].tag = argv[i] + 1;
>  	    tag_ops[tag_ops_count].remove = (argv[i][0] == '-');
>  	    tag_ops_count++;
> diff --git a/test/tagging b/test/tagging
> index e4782ed..980ff92 100755
> --- a/test/tagging
> +++ b/test/tagging
> @@ -46,4 +46,8 @@ test_expect_equal "$output" "\
>  thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 unread)
>  thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag4 unread)"
>  
> +test_expect_code 1 "Empty tag names" 'notmuch tag + One'
> +
> +test_expect_code 1 "Tag name beginning with -" 'notmuch tag +- One'
> +
>  test_done
> -- 
> 1.7.10
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch