[PATCH] python: bind add_property/remove_property and related methods

[PATCH] python: bind add_property/remove_property and related methods

[VA]

[-- Attachment #1: Type: text/plain, Size: 60 bytes --]

These methods were simply missing from the Python bindings.

[-- Attachment #2: 0001-python-bind-add_property-remove_property-and-related.patch --]
[-- Type: text/x-patch, Size: 5565 bytes --]

From 47dcf1659377f1ec8a237fbe474a5412123d0aa1 Mon Sep 17 00:00:00 2001
From: hydrargyrum <dev@indigo.re>
Date: Sun, 27 Jan 2019 09:43:57 +0100
Subject: [PATCH] python: bind add_property/remove_property and related methods

Methods for manipulating properties were not bound in Python.
---
 bindings/python/notmuch/message.py | 117 +++++++++++++++++++++++++++++
 1 file changed, 117 insertions(+)

diff --git a/bindings/python/notmuch/message.py b/bindings/python/notmuch/message.py
index 6e32b5f7..d5378b17 100644
--- a/bindings/python/notmuch/message.py
+++ b/bindings/python/notmuch/message.py
@@ -144,6 +144,28 @@ class Message(Python3StringMixIn):
     _properties_move_to_next.argtypes = [NotmuchMessagePropertiesP]
     _properties_move_to_next.restype = None
 
+    """notmuch_message_add_property"""
+    _add_property = nmlib.notmuch_message_add_property
+    _add_property.argtypes = [NotmuchMessageP, c_char_p, c_char_p]
+    _add_property.restype = c_int
+
+    """notmuch_message_remove_property"""
+    _remove_property = nmlib.notmuch_message_remove_property
+    _remove_property.argtypes = [NotmuchMessageP, c_char_p, c_char_p]
+    _remove_property.restype = c_int
+
+    """notmuch_message_remove_all_properties"""
+    _remove_all_properties = nmlib.notmuch_message_remove_all_properties
+    _remove_all_properties.argtypes = [NotmuchMessageP, c_char_p]
+    _remove_all_properties.restype = c_int
+
+    """notmuch_message_remove_all_properties_with_prefix"""
+    _remove_all_properties_with_prefix = \
+        nmlib.notmuch_message_remove_all_properties_with_prefix
+    _remove_all_properties_with_prefix.argtypes = \
+        [NotmuchMessageP, c_char_p]
+    _remove_all_properties_with_prefix.restype = c_int
+
     #Constants: Flags that can be set/get with set_flag
     FLAG = Enum(['MATCH'])
 
@@ -513,6 +535,101 @@ class Message(Python3StringMixIn):
             yield key.decode("utf-8"), value.decode("utf-8")
             Message._properties_move_to_next(properties)
 
+    def add_property(self, key, value):
+        """ Add a (key,value) pair to a message
+
+        :param key: The name of the property (may not contain an
+            '=' character).
+        :param value: The value of the property to add.
+        :returns: STATUS.SUCCESS if the property was successfully added.
+                  Raises an exception otherwise.
+        :raises: :exc:`NotInitializedError` if message has not been
+                 initialized
+        """
+        if not self._msg:
+            raise NotInitializedError()
+
+        status = self._add_property(self._msg, _str(key), _str(value))
+
+        # bail out on failure
+        if status != STATUS.SUCCESS:
+            raise NotmuchError(status)
+
+        return STATUS.SUCCESS
+
+    def remove_property(self, key, value):
+        """ Remove a (key,value) pair from a message
+
+        It is not an error to remove a non-existant (key,value) pair.
+
+        :param key: The name of the property (may not contain an
+            '=' character).
+        :param value: The value of the property to remove.
+        :returns: STATUS.SUCCESS if the property was successfully removed.
+                  Raises an exception otherwise.
+        :raises: :exc:`NotInitializedError` if message has not been
+                 initialized
+        """
+        if not self._msg:
+            raise NotInitializedError()
+
+        status = self._remove_property(self._msg, _str(key), _str(value))
+
+        # bail out on failure
+        if status != STATUS.SUCCESS:
+            raise NotmuchError(status)
+
+        return STATUS.SUCCESS
+
+    def remove_all_properties(self, key):
+        """ Remove all (key,value) pairs from the given message
+
+        :param key: key to delete properties for. If None, delete properties
+            properties for all keys
+        :returns: STATUS.SUCCESS if the properties were successfully removed.
+                  Raises an exception otherwise.
+        :raises: :exc:`NotInitializedError` if message has not been
+                 initialized
+        """
+        if not self._msg:
+            raise NotInitializedError()
+
+        if key is None:
+            status = self._remove_all_properties(self._msg, None)
+        else:
+            status = self._remove_all_properties(self._msg, _str(key))
+
+        # bail out on failure
+        if status != STATUS.SUCCESS:
+            raise NotmuchError(status)
+
+        return STATUS.SUCCESS
+
+    def remove_all_properties_with_prefix(self, key):
+        """ Remove all (prefix*,value) pairs from the given message
+
+        :param key: delete properties with keys that start with prefix.
+            If None, delete all properties.
+        :returns: STATUS.SUCCESS if the properties were successfully removed.
+                  Raises an exception otherwise.
+        :raises: :exc:`NotInitializedError` if message has not been
+                 initialized
+        """
+        if not self._msg:
+            raise NotInitializedError()
+
+        if key is None:
+            status = self._remove_all_properties_with_prefix(self._msg, None)
+        else:
+            status = self._remove_all_properties_with_prefix(self._msg,
+                                                             _str(key))
+
+        # bail out on failure
+        if status != STATUS.SUCCESS:
+            raise NotmuchError(status)
+
+        return STATUS.SUCCESS
+
     def freeze(self):
         """Freezes the current state of 'message' within the database
 
-- 
2.20.1

Re: [PATCH] python: bind add_property/remove_property and related methods

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 1663 bytes --]

Hi VA--

On Sat 2019-06-08 17:37:10 +0200, VA wrote:
> These methods were simply missing from the Python bindings.
> From 47dcf1659377f1ec8a237fbe474a5412123d0aa1 Mon Sep 17 00:00:00 2001
> From: hydrargyrum <dev@indigo.re>
> Date: Sun, 27 Jan 2019 09:43:57 +0100
> Subject: [PATCH] python: bind add_property/remove_property and related methods
>
> Methods for manipulating properties were not bound in Python.

Thanks for offering this patch.

I think we've thus far deliberately avoided exposing property
manipulation from python because properties have (by convention i think)
tended to be things that are set or cleared only by the notmuch indexer
itself.  (see notmuch-properties(7) for description of those properties)

We do expose this functionality in the library, so it's not the end of
the world to expose it in the python bindings, but i do worry a little
bit about encouraging people to fiddle with markers set by the indexer.

as nomtuch-properties(7) says:

       Extensions  to  notmuch  which make use of properties are encouraged to
       report the specific properties used to the upstream notmuch project, as
       a way of avoiding collisions in the property namespace.

So: no objections from me to the idea of the functionality here -- if
someone thinks this is useful for some project, i hope they'll speak up
for how they plan to use it.

That said, i'd love it if at least the docstrings for functions which
modify properties included some concise, thoughtful suggestion that
encourages collaboration with the community and urges caution when
potentially tampering with properties used by other parts of the
ecosystem.

        --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH] python: bind add_property/remove_property and related methods

[VA]

Le 09/06/2019 à 21:58, Daniel Kahn Gillmor a écrit :
> We do expose this functionality in the library, so it's not the end of
> the world to expose it in the python bindings, but i do worry a little
> bit about encouraging people to fiddle with markers set by the indexer.

Conflicts could be easily avoided by recommending a prefix like 
"x-<project>-" to property names (see link below).

> So: no objections from me to the idea of the functionality here -- if
> someone thinks this is useful for some project, i hope they'll speak up
> for how they plan to use it.

I'm writing a MUA, and am using properties to store an excerpt of each 
message, so the MUA can show a preview of each message (like the Gmail 
web app, or Apple mail client).

Some pleople recently suggested sync info or other MUA metadata:
https://notmuchmail.org/pipermail/notmuch/2019/027403.html

> That said, i'd love it if at least the docstrings for functions which
> modify properties included some concise, thoughtful suggestion that
> encourages collaboration with the community and urges caution when
> potentially tampering with properties used by other parts of the
> ecosystem.

I took the documentation from notmuch.h but only edited to be consistent 
with Python. Those warnings should also be added in notmuch.h, right?

Re: [PATCH] python: bind add_property/remove_property and related methods

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 2276 bytes --]

On Mon 2019-06-10 14:55:48 +0200, VA wrote:
> Le 09/06/2019 à 21:58, Daniel Kahn Gillmor a écrit :
>> We do expose this functionality in the library, so it's not the end of
>> the world to expose it in the python bindings, but i do worry a little
>> bit about encouraging people to fiddle with markers set by the indexer.
>
> Conflicts could be easily avoided by recommending a prefix like 
> "x-<project>-" to property names (see link below).

That makes sense -- we'd also want to encourage projects to note their
project name someplace central; having a namespace registry helps to
avoid collisions, and also helps to advertise the projects that derive
from notmuch :)

maybe update the wiki for that?

That said, if a given property is concretely useful, i don't want to
have it languish permanently in the x-<project>- namespace, or to force
users to adopt that particular MUA to use it -- i'd like to make it
available to all the notmuch-derived mail user agents.  Maybe that
happens only when the feature gets added to libnotmuch itself?  How do
we incentivize projects into getting that kind of widely-useful feature
into the libnotmuch mainstream instead of having it as a differentiating
factor for their specific MUA?

how should we think about managing these parts of the ecosystem?

> I'm writing a MUA, and am using properties to store an excerpt of each 
> message, so the MUA can show a preview of each message (like the Gmail 
> web app, or Apple mail client).

cool, this sounds super useful.  if you have a chance to write up how
you're generating/selecting this extract, i'd love to read more.

> Some pleople recently suggested sync info or other MUA metadata:
> https://notmuchmail.org/pipermail/notmuch/2019/027403.html

Thanks for the pointer, i'd missed that thread back in February.

I agree that there's sufficient interest to take this use case more
seriously.

> I took the documentation from notmuch.h but only edited to be consistent 
> with Python. Those warnings should also be added in notmuch.h, right?

Yeah, that's probably a good idea.  Is it asking too much to ask you to
update notmuch.h as well when you resubmit this series?

Thanks for helping make notmuch better!

       --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH] python: bind add_property/remove_property and related methods

[VA]

Le 11/06/2019 à 11:31, Daniel Kahn Gillmor a écrit :
> That makes sense -- we'd also want to encourage projects to note their
> project name someplace central; having a namespace registry helps to
> avoid collisions, and also helps to advertise the projects that derive
> from notmuch :)
> 
> maybe update the wiki for that?

The wiki would serve to advertise each projects interests, and if some 
other project has a common interest, they could get together to 
standardize it in the interest of both projects?

> Maybe that
> happens only when the feature gets added to libnotmuch itself?

IMHO, libnotmuch should stay focused on the core: indexing and tagging, 
avoid becoming bloated by staying minimal, doing one thing well. Else, 
it would not deserve the "notmuch" name anymore!

However, maybe this could be in some extras, maybe a separate 
notmuch-extensions library.

> How do
> we incentivize projects into getting that kind of widely-useful feature
> into the libnotmuch mainstream instead of having it as a differentiating
> factor for their specific MUA?
> 
> how should we think about managing these parts of the ecosystem?

I'm too new to libnotmuch to answer, maybe other people have an opinion?

>> I'm writing a MUA, and am using properties to store an excerpt of each
>> message, so the MUA can show a preview of each message (like the Gmail
>> web app, or Apple mail client).
> 
> cool, this sounds super useful.  if you have a chance to write up how
> you're generating/selecting this extract, i'd love to read more.

For messages with a plain text part, I'm taking the first 100 chars. If 
there's no plain text part but an HTML part, I'm using some random 
html2text library (https://github.com/Alir3z4/html2text) and take the 
first 100 chars.

>> I took the documentation from notmuch.h but only edited to be consistent
>> with Python. Those warnings should also be added in notmuch.h, right?
> 
> Yeah, that's probably a good idea.  Is it asking too much to ask you to
> update notmuch.h as well when you resubmit this series?

Here's what we could add:

As a general rule, an application MUST prefix their own property names 
with "x-<project>-". It is recommended to report an application's 
properties on the notmuch wiki, to open collaboration with other 
projects having common use cases, ultimately opening to standardization 
outside a project's namespace.

Re: [PATCH] python: bind add_property/remove_property and related methods

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 2537 bytes --]

On Fri 2019-06-14 22:34:16 +0200, VA wrote:
> The wiki would serve to advertise each projects interests, and if some 
> other project has a common interest, they could get together to 
> standardize it in the interest of both projects?

Makes sense to me.  Each one then gets to deal with the legacy of having
the old "x-<project>-foo" property and new standard form "foo", which is
kind of annoying bookkeeping work, but maybe not too hard to do.  I'm
sure someone can write a converter script pretty simply once
"x-<project>-foo" is fully deprecated in such a transition.

> IMHO, libnotmuch should stay focused on the core: indexing and tagging, 
> avoid becoming bloated by staying minimal, doing one thing well. Else, 
> it would not deserve the "notmuch" name anymore!
>
> However, maybe this could be in some extras, maybe a separate 
> notmuch-extensions library.

Hm, i'm not so worried about keeping the semantics of the "notmuch" name
:P

If some useful feature can happen most efficiently at indexing time, and
the index is built by the library, i think the ecosystem is best served
by making sure that libnotmuch can just do it directly.

> For messages with a plain text part, I'm taking the first 100 chars. If 
> there's no plain text part but an HTML part, I'm using some random 
> html2text library (https://github.com/Alir3z4/html2text) and take the 
> first 100 chars.

makes sense, thanks for the simple and straightforward description.  I
assume if the message has neither a text/plain nor a text/html part,
then no property is added.  And for messages with multiple text/plain
parts, you just take the first text/plain part encountered in a
depth-first traversal of the MIME tree?

> Here's what we could add:
>
> As a general rule, an application MUST prefix their own property names 
> with "x-<project>-". It is recommended to report an application's 
> properties on the notmuch wiki, to open collaboration with other 
> projects having common use cases, ultimately opening to standardization 
> outside a project's namespace.

I like this text.

As a minor nit-pick, I'd change the MUST to a SHOULD if we're using
RFC-2119-style requirements keywords here, since i can imagine an
application developer talking here on the notmuch list and coming to
consensus in some particular use case that a given property should not
be project-specific.  iow, they need to know *why* they're not following
the recommendation.

Thanks for writing this up!

    --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH] python: bind add_property/remove_property and related methods

[Daniel Kahn Gillmor]

On Sun 2019-06-16 02:52:52 +0300, Daniel Kahn Gillmor wrote:
> On Fri 2019-06-14 22:34:16 +0200, VA wrote:
>> As a general rule, an application MUST prefix their own property names 
>> with "x-<project>-". It is recommended to report an application's 
>> properties on the notmuch wiki, to open collaboration with other 
>> projects having common use cases, ultimately opening to standardization 
>> outside a project's namespace.
>
> I like this text.

I note that in id:874l92gcy2.fsf@tethera.net, Bremner points out:

>>> BTW, notmuch is currently using . as a namespace separator, so this
>>> would look more like "x-project."

(as opposed to "x-project-' , that is. note the difference in
punctuation)

    --dkg