[PATCH 1/2] tag: Disallow adding malformed tags to messages

[PATCH 1/2] tag: Disallow adding malformed tags to messages

[Austin Clements]

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.
---
 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

[PATCH 2/2] News for restrictions on tag names

[Austin Clements]

---
 NEWS |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/NEWS b/NEWS
index 2b50ba3..ebd5c1e 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,17 @@
+Notmuch 0.15 (YYYY-MM-DD)
+=========================
+
+Command-Line Interface
+----------------------
+
+Empty tag names and tags beginning with "-" are deprecated
+
+  Such tags have been a frequent source of confusion and cause
+  (sometimes unresolvable) conflicts with other syntax.  notmuch tag
+  no longer allows such tags to be added to messages.  Removing such
+  tags continues to be supported to allow cleanup of existing tags,
+  but may be removed in a future release.
+
 Notmuch 0.14 (2012-08-20)
 =========================
 
-- 
1.7.10

Re: [PATCH 1/2] tag: Disallow adding malformed tags to messages

[Tomi Ollila]

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

Re: [PATCH 1/2] tag: Disallow adding malformed tags to messages

[Ethan Glasser-Camp]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> LGTM (NEWS too)

Yep! Removing needs-review.

Ethan

Re: [PATCH 1/2] tag: Disallow adding malformed tags to messages

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:

> 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.

Pushed both,

d