[PATCH 06/11] lib: Internal support for querying and creating ghost messages
On Mon, 06 Oct 2014, David Bremner wrote: > Austin Clements writes: > >> >> I'm used to reading this stuff, so either way is fine with me. Do we >> have bit set / clear / read macros? >> > > I guess not. the things we have in query.cc are related but different. I added some macros for doing this to notmuch-private.h and converted the other bit twiddling for message flags to use these. >>> > + else if (*i == "Tghost") >>> > + message->flags |= (1 << NOTMUCH_MESSAGE_FLAG_GHOST); >>> > + else >>> >>> It makes me faintly unhappy to have the prefix hardcoded here. >>> Not sure if there is a sensible solution. >> >> I agree, but I also don't want to construct the test string every time >> or deconstruct the term string every time. I could move the "T" >> prefix string to a #define and use that both here and in >> BOOLEAN_PREFIX_INTERNAL, but that solution may be worse than the >> problem. What do you think? > > Maybe just a comment to point to BOOLEAN_PREFIX_INTERNAL. > > Or maybe define a macro right beside BOOLEAN_PREFIX_INTERNAL like > > #define ADD_TYPE_PREFIX(s) "T" s > > At least then the duplication is all in one place. A #define by BOOLEAN_PREFIX_INTERNAL won't help because BOOLEAN_PREFIX_INTERNAL lives in database.cc and this code is in message.cc. I would have to put the #define in one of the private headers, but I could use it in BOOLEAN_PREFIX_INTERNAL so there wouldn't be any duplication of the "T" string. I added a comment pointing to BOOLEAN_PREFIX_INTERNAL. Maybe that's enough? I'll post v2 later today, when I can run the test suite (currently running on battery).
Re: [PATCH 06/11] lib: Internal support for querying and creating ghost messages
On Mon, 06 Oct 2014, David Bremner wrote: > Austin Clements writes: > >> >> I'm used to reading this stuff, so either way is fine with me. Do we >> have bit set / clear / read macros? >> > > I guess not. the things we have in query.cc are related but different. I added some macros for doing this to notmuch-private.h and converted the other bit twiddling for message flags to use these. >>> > + else if (*i == "Tghost") >>> > + message->flags |= (1 << NOTMUCH_MESSAGE_FLAG_GHOST); >>> > + else >>> >>> It makes me faintly unhappy to have the prefix hardcoded here. >>> Not sure if there is a sensible solution. >> >> I agree, but I also don't want to construct the test string every time >> or deconstruct the term string every time. I could move the "T" >> prefix string to a #define and use that both here and in >> BOOLEAN_PREFIX_INTERNAL, but that solution may be worse than the >> problem. What do you think? > > Maybe just a comment to point to BOOLEAN_PREFIX_INTERNAL. > > Or maybe define a macro right beside BOOLEAN_PREFIX_INTERNAL like > > #define ADD_TYPE_PREFIX(s) "T" s > > At least then the duplication is all in one place. A #define by BOOLEAN_PREFIX_INTERNAL won't help because BOOLEAN_PREFIX_INTERNAL lives in database.cc and this code is in message.cc. I would have to put the #define in one of the private headers, but I could use it in BOOLEAN_PREFIX_INTERNAL so there wouldn't be any duplication of the "T" string. I added a comment pointing to BOOLEAN_PREFIX_INTERNAL. Maybe that's enough? I'll post v2 later today, when I can run the test suite (currently running on battery). ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 06/11] lib: Internal support for querying and creating ghost messages
Austin Clements writes: > > I'm used to reading this stuff, so either way is fine with me. Do we > have bit set / clear / read macros? > I guess not. the things we have in query.cc are related but different. >> > + else if (*i == "Tghost") >> > + message->flags |= (1 << NOTMUCH_MESSAGE_FLAG_GHOST); >> > + else >> >> It makes me faintly unhappy to have the prefix hardcoded here. >> Not sure if there is a sensible solution. > > I agree, but I also don't want to construct the test string every time > or deconstruct the term string every time. I could move the "T" > prefix string to a #define and use that both here and in > BOOLEAN_PREFIX_INTERNAL, but that solution may be worse than the > problem. What do you think? Maybe just a comment to point to BOOLEAN_PREFIX_INTERNAL. Or maybe define a macro right beside BOOLEAN_PREFIX_INTERNAL like #define ADD_TYPE_PREFIX(s) "T" s At least then the duplication is all in one place.
Re: [PATCH 06/11] lib: Internal support for querying and creating ghost messages
Austin Clements writes: > > I'm used to reading this stuff, so either way is fine with me. Do we > have bit set / clear / read macros? > I guess not. the things we have in query.cc are related but different. >> > + else if (*i == "Tghost") >> > + message->flags |= (1 << NOTMUCH_MESSAGE_FLAG_GHOST); >> > + else >> >> It makes me faintly unhappy to have the prefix hardcoded here. >> Not sure if there is a sensible solution. > > I agree, but I also don't want to construct the test string every time > or deconstruct the term string every time. I could move the "T" > prefix string to a #define and use that both here and in > BOOLEAN_PREFIX_INTERNAL, but that solution may be worse than the > problem. What do you think? Maybe just a comment to point to BOOLEAN_PREFIX_INTERNAL. Or maybe define a macro right beside BOOLEAN_PREFIX_INTERNAL like #define ADD_TYPE_PREFIX(s) "T" s At least then the duplication is all in one place. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 06/11] lib: Internal support for querying and creating ghost messages
Quoth David Bremner on Oct 05 at 10:30 am: > Austin Clements writes: > > > + message->flags &= ~(1 << NOTMUCH_MESSAGE_FLAG_GHOST); > > What do you think about using bit set / clear / read macros? I don't > insist, but I wonder if it would make this part more readable. I'm used to reading this stuff, so either way is fine with me. Do we have bit set / clear / read macros? > > + else if (*i == "Tghost") > > + message->flags |= (1 << NOTMUCH_MESSAGE_FLAG_GHOST); > > + else > > It makes me faintly unhappy to have the prefix hardcoded here. > Not sure if there is a sensible solution. I agree, but I also don't want to construct the test string every time or deconstruct the term string every time. I could move the "T" prefix string to a #define and use that both here and in BOOLEAN_PREFIX_INTERNAL, but that solution may be worse than the problem. What do you think?
Re: [PATCH 06/11] lib: Internal support for querying and creating ghost messages
Quoth David Bremner on Oct 05 at 10:30 am: > Austin Clements writes: > > > + message->flags &= ~(1 << NOTMUCH_MESSAGE_FLAG_GHOST); > > What do you think about using bit set / clear / read macros? I don't > insist, but I wonder if it would make this part more readable. I'm used to reading this stuff, so either way is fine with me. Do we have bit set / clear / read macros? > > + else if (*i == "Tghost") > > + message->flags |= (1 << NOTMUCH_MESSAGE_FLAG_GHOST); > > + else > > It makes me faintly unhappy to have the prefix hardcoded here. > Not sure if there is a sensible solution. I agree, but I also don't want to construct the test string every time or deconstruct the term string every time. I could move the "T" prefix string to a #define and use that both here and in BOOLEAN_PREFIX_INTERNAL, but that solution may be worse than the problem. What do you think? ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 06/11] lib: Internal support for querying and creating ghost messages
Austin Clements writes: > + message->flags &= ~(1 << NOTMUCH_MESSAGE_FLAG_GHOST); What do you think about using bit set / clear / read macros? I don't insist, but I wonder if it would make this part more readable. > + else if (*i == "Tghost") > + message->flags |= (1 << NOTMUCH_MESSAGE_FLAG_GHOST); > + else It makes me faintly unhappy to have the prefix hardcoded here. Not sure if there is a sensible solution.
Re: [PATCH 06/11] lib: Internal support for querying and creating ghost messages
Austin Clements writes: > + message->flags &= ~(1 << NOTMUCH_MESSAGE_FLAG_GHOST); What do you think about using bit set / clear / read macros? I don't insist, but I wonder if it would make this part more readable. > + else if (*i == "Tghost") > + message->flags |= (1 << NOTMUCH_MESSAGE_FLAG_GHOST); > + else It makes me faintly unhappy to have the prefix hardcoded here. Not sure if there is a sensible solution. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 06/11] lib: Internal support for querying and creating ghost messages
From: Austin Clements
This updates the message abstraction to support ghost messages: it
adds a message flag that distinguishes regular messages from ghost
messages, and an internal function for initializing a newly created
(blank) message as a ghost message.
---
lib/message.cc| 50 --
lib/notmuch-private.h | 4
lib/notmuch.h | 9 -
3 files changed, 60 insertions(+), 3 deletions(-)
diff --git a/lib/message.cc b/lib/message.cc
index 38bc929..ad832cf 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -39,6 +39,9 @@ struct visible _notmuch_message {
notmuch_message_file_t *message_file;
notmuch_message_list_t *replies;
unsigned long flags;
+/* For flags that are initialized on-demand, lazy_flags indicates
+ * if each flag has been initialized. */
+unsigned long lazy_flags;
Xapian::Document doc;
Xapian::termcount termpos;
@@ -99,6 +102,7 @@ _notmuch_message_create_for_document (const void
*talloc_owner,
message->frozen = 0;
message->flags = 0;
+message->lazy_flags = 0;
/* Each of these will be lazily created as needed. */
message->message_id = NULL;
@@ -192,7 +196,7 @@ _notmuch_message_create (const void *talloc_owner,
*
* There is already a document with message ID 'message_id' in the
* database. The returned message can be used to query/modify the
- * document.
+ * document. The message may be a ghost message.
*
* NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND:
*
@@ -305,6 +309,7 @@ _notmuch_message_ensure_metadata (notmuch_message_t
*message)
const char *thread_prefix = _find_prefix ("thread"),
*tag_prefix = _find_prefix ("tag"),
*id_prefix = _find_prefix ("id"),
+ *type_prefix = _find_prefix ("type"),
*filename_prefix = _find_prefix ("file-direntry"),
*replyto_prefix = _find_prefix ("replyto");
@@ -337,10 +342,23 @@ _notmuch_message_ensure_metadata (notmuch_message_t
*message)
message->message_id =
_notmuch_message_get_term (message, i, end, id_prefix);
+/* Get document type */
+assert (strcmp (id_prefix, type_prefix) < 0);
+if (! (message->lazy_flags & (1 << NOTMUCH_MESSAGE_FLAG_GHOST))) {
+ i.skip_to (type_prefix);
+ if (*i == "Tmail")
+ message->flags &= ~(1 << NOTMUCH_MESSAGE_FLAG_GHOST);
+ else if (*i == "Tghost")
+ message->flags |= (1 << NOTMUCH_MESSAGE_FLAG_GHOST);
+ else
+ INTERNAL_ERROR ("Message without type term");
+ message->lazy_flags |= (1 << NOTMUCH_MESSAGE_FLAG_GHOST);
+}
+
/* Get filename list. Here we get only the terms. We lazily
* expand them to full file names when needed in
* _notmuch_message_ensure_filename_list. */
-assert (strcmp (id_prefix, filename_prefix) < 0);
+assert (strcmp (type_prefix, filename_prefix) < 0);
if (!message->filename_term_list && !message->filename_list)
message->filename_term_list =
_notmuch_database_get_terms_with_prefix (message, i, end,
@@ -371,6 +389,11 @@ _notmuch_message_invalidate_metadata (notmuch_message_t
*message,
message->tag_list = NULL;
}
+if (strcmp ("type", prefix_name) == 0) {
+ message->flags &= ~(1 << NOTMUCH_MESSAGE_FLAG_GHOST);
+ message->lazy_flags &= ~(1 << NOTMUCH_MESSAGE_FLAG_GHOST);
+}
+
if (strcmp ("file-direntry", prefix_name) == 0) {
talloc_free (message->filename_term_list);
talloc_free (message->filename_list);
@@ -869,6 +892,10 @@ notmuch_bool_t
notmuch_message_get_flag (notmuch_message_t *message,
notmuch_message_flag_t flag)
{
+if (flag == NOTMUCH_MESSAGE_FLAG_GHOST &&
+ ! (message->lazy_flags & (1 << flag)))
+ _notmuch_message_ensure_metadata (message);
+
return message->flags & (1 << flag);
}
@@ -880,6 +907,7 @@ notmuch_message_set_flag (notmuch_message_t *message,
message->flags |= (1 << flag);
else
message->flags &= ~(1 << flag);
+message->lazy_flags |= (1 << flag);
}
time_t
@@ -989,6 +1017,24 @@ _notmuch_message_delete (notmuch_message_t *message)
return NOTMUCH_STATUS_SUCCESS;
}
+/* Transform a blank message into a ghost message. The caller must
+ * _notmuch_message_sync the message. */
+notmuch_private_status_t
+_notmuch_message_initialize_ghost (notmuch_message_t *message,
+ const char *thread_id)
+{
+notmuch_private_status_t status;
+
+status = _notmuch_message_add_term (message, "type", "ghost");
+if (status)
+ return status;
+status = _notmuch_message_add_term (message, "thread", thread_id);
+if (status)
+ return status;
+
+return NOTMUCH_PRIVATE_STATUS_SUCCESS;
+}
+
/* Ensure that 'message' is not holding any file object open. Future
* calls to various functions will still automatically open the
* message file as needed.
diff --git a/lib/n
[PATCH 06/11] lib: Internal support for querying and creating ghost messages
From: Austin Clements
This updates the message abstraction to support ghost messages: it
adds a message flag that distinguishes regular messages from ghost
messages, and an internal function for initializing a newly created
(blank) message as a ghost message.
---
lib/message.cc| 50 --
lib/notmuch-private.h | 4
lib/notmuch.h | 9 -
3 files changed, 60 insertions(+), 3 deletions(-)
diff --git a/lib/message.cc b/lib/message.cc
index 38bc929..ad832cf 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -39,6 +39,9 @@ struct visible _notmuch_message {
notmuch_message_file_t *message_file;
notmuch_message_list_t *replies;
unsigned long flags;
+/* For flags that are initialized on-demand, lazy_flags indicates
+ * if each flag has been initialized. */
+unsigned long lazy_flags;
Xapian::Document doc;
Xapian::termcount termpos;
@@ -99,6 +102,7 @@ _notmuch_message_create_for_document (const void
*talloc_owner,
message->frozen = 0;
message->flags = 0;
+message->lazy_flags = 0;
/* Each of these will be lazily created as needed. */
message->message_id = NULL;
@@ -192,7 +196,7 @@ _notmuch_message_create (const void *talloc_owner,
*
* There is already a document with message ID 'message_id' in the
* database. The returned message can be used to query/modify the
- * document.
+ * document. The message may be a ghost message.
*
* NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND:
*
@@ -305,6 +309,7 @@ _notmuch_message_ensure_metadata (notmuch_message_t
*message)
const char *thread_prefix = _find_prefix ("thread"),
*tag_prefix = _find_prefix ("tag"),
*id_prefix = _find_prefix ("id"),
+ *type_prefix = _find_prefix ("type"),
*filename_prefix = _find_prefix ("file-direntry"),
*replyto_prefix = _find_prefix ("replyto");
@@ -337,10 +342,23 @@ _notmuch_message_ensure_metadata (notmuch_message_t
*message)
message->message_id =
_notmuch_message_get_term (message, i, end, id_prefix);
+/* Get document type */
+assert (strcmp (id_prefix, type_prefix) < 0);
+if (! (message->lazy_flags & (1 << NOTMUCH_MESSAGE_FLAG_GHOST))) {
+ i.skip_to (type_prefix);
+ if (*i == "Tmail")
+ message->flags &= ~(1 << NOTMUCH_MESSAGE_FLAG_GHOST);
+ else if (*i == "Tghost")
+ message->flags |= (1 << NOTMUCH_MESSAGE_FLAG_GHOST);
+ else
+ INTERNAL_ERROR ("Message without type term");
+ message->lazy_flags |= (1 << NOTMUCH_MESSAGE_FLAG_GHOST);
+}
+
/* Get filename list. Here we get only the terms. We lazily
* expand them to full file names when needed in
* _notmuch_message_ensure_filename_list. */
-assert (strcmp (id_prefix, filename_prefix) < 0);
+assert (strcmp (type_prefix, filename_prefix) < 0);
if (!message->filename_term_list && !message->filename_list)
message->filename_term_list =
_notmuch_database_get_terms_with_prefix (message, i, end,
@@ -371,6 +389,11 @@ _notmuch_message_invalidate_metadata (notmuch_message_t
*message,
message->tag_list = NULL;
}
+if (strcmp ("type", prefix_name) == 0) {
+ message->flags &= ~(1 << NOTMUCH_MESSAGE_FLAG_GHOST);
+ message->lazy_flags &= ~(1 << NOTMUCH_MESSAGE_FLAG_GHOST);
+}
+
if (strcmp ("file-direntry", prefix_name) == 0) {
talloc_free (message->filename_term_list);
talloc_free (message->filename_list);
@@ -869,6 +892,10 @@ notmuch_bool_t
notmuch_message_get_flag (notmuch_message_t *message,
notmuch_message_flag_t flag)
{
+if (flag == NOTMUCH_MESSAGE_FLAG_GHOST &&
+ ! (message->lazy_flags & (1 << flag)))
+ _notmuch_message_ensure_metadata (message);
+
return message->flags & (1 << flag);
}
@@ -880,6 +907,7 @@ notmuch_message_set_flag (notmuch_message_t *message,
message->flags |= (1 << flag);
else
message->flags &= ~(1 << flag);
+message->lazy_flags |= (1 << flag);
}
time_t
@@ -989,6 +1017,24 @@ _notmuch_message_delete (notmuch_message_t *message)
return NOTMUCH_STATUS_SUCCESS;
}
+/* Transform a blank message into a ghost message. The caller must
+ * _notmuch_message_sync the message. */
+notmuch_private_status_t
+_notmuch_message_initialize_ghost (notmuch_message_t *message,
+ const char *thread_id)
+{
+notmuch_private_status_t status;
+
+status = _notmuch_message_add_term (message, "type", "ghost");
+if (status)
+ return status;
+status = _notmuch_message_add_term (message, "thread", thread_id);
+if (status)
+ return status;
+
+return NOTMUCH_PRIVATE_STATUS_SUCCESS;
+}
+
/* Ensure that 'message' is not holding any file object open. Future
* calls to various functions will still automatically open the
* message file as needed.
diff --gi
