From: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
To: Notmuch Mail <notmuch@notmuchmail.org>
Subject: [PATCH] n_m_remove_property(msg, key,
 NULL) should removes all properties with key
Date: Fri,  8 Jul 2016 06:36:50 +0200
Message-Id: <1467952610-27015-1-git-send-email-dkg@fifthhorseman.net>
In-Reply-To: <1465779955-5539-4-git-send-email-david@tethera.net>
References: <1465779955-5539-4-git-send-email-david@tethera.net>
Precedence: list

We should not require value to be non-NULL when there is a perfectly
sensible semantic:

    notmuch_message_remove_property(msg, key, NULL)

should mean "remove all properties with the given key from msg"
---
 lib/message-property.cc | 28 ++++++++++++++++++++--------
 lib/notmuch.h           |  5 +++--
 2 files changed, 23 insertions(+), 10 deletions(-)

This should apply on top of bremner's "properties" series, making the
semantics slightly more useful.

diff --git a/lib/message-property.cc b/lib/message-property.cc
index 4fa6cac..338f3fb 100644
--- a/lib/message-property.cc
+++ b/lib/message-property.cc
@@ -48,22 +48,34 @@ _notmuch_message_modify_property (notmuch_message_t *message, const char *key, c
     if (status)
 	return status;
 
-    if (key == NULL || value == NULL)
+    if (key == NULL)
 	return NOTMUCH_STATUS_NULL_POINTER;
 
     if (index (key, '='))
 	return NOTMUCH_STATUS_ILLEGAL_ARGUMENT;
 
-    term = talloc_asprintf (message, "%s=%s", key, value);
+    if (value == NULL)
+    {
+	if (!delete_it)
+	    return NOTMUCH_STATUS_NULL_POINTER;
 
-    if (delete_it)
-	private_status = _notmuch_message_remove_term (message, "property", term);
+	term = talloc_asprintf (message, "%s%s=", _find_prefix ("property"), key);
+	_notmuch_message_remove_terms (message, term);
+    }
     else
-	private_status = _notmuch_message_add_term (message, "property", term);
+    {
+	term = talloc_asprintf (message, "%s=%s", key, value);
+
+	if (delete_it)
+	    private_status = _notmuch_message_remove_term (message, "property", term);
+	else
+	    private_status = _notmuch_message_add_term (message, "property", term);
+
+	if (private_status)
+	    return COERCE_STATUS (private_status,
+				  "Unhandled error modifying message property");
+    }
 
-    if (private_status)
-	return COERCE_STATUS (private_status,
-			      "Unhandled error modifying message property");
     if (! _notmuch_message_frozen (message))
 	_notmuch_message_sync (message);
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index f6bad67..0d667f5 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1698,11 +1698,12 @@ notmuch_message_add_property (notmuch_message_t *message, const char *key, const
 /**
  * Remove a (key,value) pair from a message.
  *
- * It is not an error to remove a non-existant (key,value) pair
+ * It is not an error to remove a non-existant (key,value) pair.  If
+ * *value* is NULL, remove all properties with the given key.
  *
  * @returns
  * - NOTMUCH_STATUS_ILLEGAL_ARGUMENT: *key* may not contain an '=' character.
- * - NOTMUCH_STATUS_NULL_POINTER: Neither *key* nor *value* may be NULL.
+ * - NOTMUCH_STATUS_NULL_POINTER: *key* may not be NULL.
  * - NOTMUCH_STATUS_SUCCESS: No error occured.
  */
 notmuch_status_t
-- 
2.8.1