Subject: Python Unicode Was: Not much database creation error
From: Martin Owens <doctormo@gmail.com>
To: Sebastian Spaeth <Sebastian@SSpaeth.de>
In-Reply-To: <8739g7gnb4.fsf@SSpaeth.de>
References: <1315110898.32058.13.camel@delen>  <87vct7t7m4.fsf@SSpaeth.de>
	<1315373219.32058.157.camel@delen>  <8739g7gnb4.fsf@SSpaeth.de>
Content-Type: multipart/mixed; boundary="=-0VlEcVbZa7gbgZ+lT5kJ"
Date: Thu, 15 Sep 2011 13:41:11 -0400
Message-ID: <1316108471.2201.24.camel@delen>
Mime-Version: 1.0
Cc: Notmuch developer list <notmuch@notmuchmail.org>,
	Paul Tagliamonte <tag@pault.ag>
Precedence: list


--=-0VlEcVbZa7gbgZ+lT5kJ
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

Hello Sebastian,

I've attached a diff for some proposed changes to help make dealing with
unicode and strings in the bindings more regular. I noticed some of the
methods were protected and others were not.

Let me know.

Best Regards, Martin Owens

On Thu, 2011-09-08 at 15:45 +0200, Sebastian Spaeth wrote:
> I have to admit, I am not deep into the libnotmuch/xapian internals,
> so
> I might be the wrong person to ask (I just do some python bindings to
> whatever libnotmuch provides). 

--=-0VlEcVbZa7gbgZ+lT5kJ
Content-Disposition: attachment; filename="unicode.patch"
Content-Type: text/x-patch; name="unicode.patch"; charset="UTF-8"
Content-Transfer-Encoding: 7bit

diff --git a/bindings/python/notmuch/database.py b/bindings/python/notmuch/database.py
index b070019..4f44de0 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -19,12 +19,11 @@ Copyright 2010 Sebastian Spaeth <Sebastian@SSpaeth.de>'
 
 import os
 from ctypes import c_int, c_char_p, c_void_p, c_uint, c_long, byref
-from notmuch.globals import nmlib, STATUS, NotmuchError, Enum
+from notmuch.globals import nmlib, STATUS, NotmuchError, Enum, _str
 from notmuch.thread import Threads
 from notmuch.message import Messages, Message
 from notmuch.tag import Tags
 
-
 class Database(object):
     """Represents a notmuch database (wraps notmuch_database_t)
 
@@ -105,7 +104,6 @@ class Database(object):
                 Database._std_db_path = self._get_user_default_db()
             path = Database._std_db_path
 
-        assert isinstance(path, basestring), 'Path must be a string or None.'
         if create == False:
             self.open(path, mode)
         else:
@@ -136,7 +134,7 @@ class Database(object):
             raise NotmuchError(message="Cannot create db, this Database() "
                                        "already has an open one.")
 
-        res = Database._create(path, Database.MODE.READ_WRITE)
+        res = Database._create(_str(path), Database.MODE.READ_WRITE)
 
         if res is None:
             raise NotmuchError(
@@ -156,9 +154,7 @@ class Database(object):
         :exception: Raises :exc:`NotmuchError` in case
                     of any failure (after printing an error message on stderr).
         """
-        if isinstance(path, unicode):
-            path = path.encode('utf-8') 
-        res = Database._open(path, mode)
+        res = Database._open(_str(path), mode)
 
         if res is None:
             raise NotmuchError(
@@ -263,12 +259,10 @@ class Database(object):
             #we got a relative path, make it absolute
             abs_dirpath = os.path.abspath(os.path.join(self.get_path(), path))
 
-        if isinstance(path, unicode):
-            path = path.encode('UTF-8')
-        dir_p = Database._get_directory(self._db, path)
+        dir_p = Database._get_directory(self._db, _str(path))
 
         # return the Directory, init it with the absolute path
-        return Directory(abs_dirpath, dir_p, self)
+        return Directory(_str(abs_dirpath), dir_p, self)
 
     def add_message(self, filename, sync_maildir_flags=False):
         """Adds a new message to the database
@@ -325,7 +319,7 @@ class Database(object):
 
         msg_p = c_void_p()
         status = nmlib.notmuch_database_add_message(self._db,
-                                                  filename,
+                                                  _str(filename),
                                                   byref(msg_p))
 
         if not status in [STATUS.SUCCESS, STATUS.DUPLICATE_MESSAGE_ID]:
@@ -394,7 +388,7 @@ class Database(object):
         # Raise a NotmuchError if not initialized
         self._verify_initialized_db()
 
-        msg_p = Database._find_message(self._db, msgid)
+        msg_p = Database._find_message(self._db, _str(msgid))
         if msg_p is None:
             return None
         return Message(msg_p, self)
@@ -404,8 +398,7 @@ class Database(object):
            with this filename.
         """
         self._verify_initialized_db()
-
-        msg_p = Database._find_message_by_filename(self._db, filename)
+        msg_p = Database._find_message_by_filename(self._db, _str(filename))
         return msg_p and Message(msg_p, self) or None
 
     def get_all_tags(self):
@@ -558,11 +551,8 @@ class Query(object):
             raise NotmuchError(STATUS.NOT_INITIALIZED)
         # create reference to parent db to keep it alive
         self._db = db
-        if isinstance(querystr, unicode):
-            # xapian takes utf-8 encoded byte arrays
-            querystr = querystr.encode('utf-8')
         # create query, return None if too little mem available
-        query_p = Query._create(db.db_p, querystr)
+        query_p = Query._create(db.db_p, _str(querystr))
         if query_p is None:
             NotmuchError(STATUS.NULL_POINTER)
         self._query = query_p
diff --git a/bindings/python/notmuch/globals.py b/bindings/python/notmuch/globals.py
index 77f2905..5c81076 100644
--- a/bindings/python/notmuch/globals.py
+++ b/bindings/python/notmuch/globals.py
@@ -98,3 +98,13 @@ class NotmuchError(Exception):
             return self.args[0]
         else:
             return STATUS.status2str(self.args[1])
+
+# C++ code expects strings to be well formatted and
+# unicode strings to have no null bytes.
+def _str(value):
+    if not isinstance(value, basestring):
+        raise TypeError("Expected str or unicode, got %s" % str(type(value)))
+    if isinstance(value, unicode):
+        return value.encode('UTF-8')
+    return str(value)
+
diff --git a/bindings/python/notmuch/message.py b/bindings/python/notmuch/message.py
index ae6ae1b..4f93a2a 100644
--- a/bindings/python/notmuch/message.py
+++ b/bindings/python/notmuch/message.py
@@ -21,7 +21,7 @@ Copyright 2010 Sebastian Spaeth <Sebastian@SSpaeth.de>'
 
 from ctypes import c_char_p, c_void_p, c_long, c_uint, c_int
 from datetime import date
-from notmuch.globals import nmlib, STATUS, NotmuchError, Enum
+from notmuch.globals import nmlib, STATUS, NotmuchError, Enum, _str
 from notmuch.tag import Tags
 from notmuch.filename import Filenames
 import sys
@@ -505,7 +505,7 @@ class Message(object):
         if self._msg is None:
             raise NotmuchError(STATUS.NOT_INITIALIZED)
 
-        status = nmlib.notmuch_message_add_tag(self._msg, tag)
+        status = nmlib.notmuch_message_add_tag(self._msg, _str(tag))
 
         # bail out on failure
         if status != STATUS.SUCCESS:
@@ -549,7 +549,7 @@ class Message(object):
         if self._msg is None:
             raise NotmuchError(STATUS.NOT_INITIALIZED)
 
-        status = nmlib.notmuch_message_remove_tag(self._msg, tag)
+        status = nmlib.notmuch_message_remove_tag(self._msg, _str(tag))
         # bail out on error
         if status != STATUS.SUCCESS:
             raise NotmuchError(status)

--=-0VlEcVbZa7gbgZ+lT5kJ--