Re: [PATCH] emacs: wash: make word-wrap bound message width

[notmuch-archives.git] / 01 / bdb16594f6b01c74c999198b7edc00af539c78

Return-Path: <aclements@csail.mit.edu>\r
X-Original-To: notmuch@notmuchmail.org\r
Delivered-To: notmuch@notmuchmail.org\r
Received: from localhost (localhost [127.0.0.1])\r
        by olra.theworths.org (Postfix) with ESMTP id 20DEF431FB6\r
        for <notmuch@notmuchmail.org>; Tue, 21 Oct 2014 18:49:22 -0700 (PDT)\r
X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
X-Spam-Flag: NO\r
X-Spam-Score: -2.3\r
X-Spam-Level: \r
X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5\r
        tests=[RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled\r
Received: from olra.theworths.org ([127.0.0.1])\r
        by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
        with ESMTP id pjKGcSE-sYOv for <notmuch@notmuchmail.org>;\r
        Tue, 21 Oct 2014 18:49:14 -0700 (PDT)\r
Received: from outgoing.csail.mit.edu (outgoing.csail.mit.edu [128.30.2.149])\r
        by olra.theworths.org (Postfix) with ESMTP id 6733A431FAE\r
        for <notmuch@notmuchmail.org>; Tue, 21 Oct 2014 18:49:14 -0700 (PDT)\r
Received: from [104.131.20.129] (helo=awakeningjr)\r
        by outgoing.csail.mit.edu with esmtpsa (TLS1.0:RSA_AES_128_CBC_SHA1:16)\r
        (Exim 4.72) (envelope-from <aclements@csail.mit.edu>)\r
        id 1Xgl33-0005Gj-S9; Tue, 21 Oct 2014 21:49:13 -0400\r
Received: from amthrax by awakeningjr with local (Exim 4.84)\r
        (envelope-from <aclements@csail.mit.edu>)\r
        id 1Xgl33-0000Ge-JH; Tue, 21 Oct 2014 21:49:13 -0400\r
Date: Tue, 21 Oct 2014 21:49:13 -0400\r
From: Austin Clements <aclements@csail.mit.edu>\r
To: Mark Walters <markwalters1009@gmail.com>\r
Subject: Re: [PATCH v2 08/12] lib: Implement ghost-based thread linking\r
Message-ID: <20141022014913.GE7970@csail.mit.edu>\r
References: <1412637438-4821-1-git-send-email-aclements@csail.mit.edu>\r
        <1412637438-4821-9-git-send-email-aclements@csail.mit.edu>\r
        <87zjcphvdu.fsf@qmul.ac.uk>\r
MIME-Version: 1.0\r
Content-Type: text/plain; charset=us-ascii\r
Content-Disposition: inline\r
In-Reply-To: <87zjcphvdu.fsf@qmul.ac.uk>\r
User-Agent: Mutt/1.5.23 (2014-03-12)\r
Cc: notmuch@notmuchmail.org\r
X-BeenThere: notmuch@notmuchmail.org\r
X-Mailman-Version: 2.1.13\r
Precedence: list\r
List-Id: "Use and development of the notmuch mail system."\r
        <notmuch.notmuchmail.org>\r
List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
        <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
List-Post: <mailto:notmuch@notmuchmail.org>\r
List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
        <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
X-List-Received-Date: Wed, 22 Oct 2014 01:49:22 -0000\r
\r
Quoth Mark Walters on Oct 22 at 12:10 am:\r
> On Tue, 07 Oct 2014, Austin Clements <aclements@csail.mit.edu> wrote:\r
> > From: Austin Clements <amdragon@mit.edu>\r
> >\r
> > This updates the thread linking code to use ghost messages instead of\r
> > user metadata to link messages into threads.\r
> >\r
> > In contrast with the old approach, this is actually correct.\r
> > Previously, thread merging updated only the thread IDs of message\r
> > documents, not thread IDs stored in user metadata.  As originally\r
> > diagnosed by Mark Walters [1] and as demonstrated by the broken\r
> > T260-thread-order test, this can cause notmuch to fail to link\r
> > messages even though they're in the same thread.  In principle the old\r
> > approach could have been fixed by updating the user metadata thread\r
> > IDs as well, but these are not indexed and hence this would have\r
> > required a full scan of all stored thread IDs.  Ghost messages solve\r
> > this problem naturally by reusing the exact same thread ID and message\r
> > ID representation and indexing as regular messages.\r
> >\r
> > Furthermore, thanks to this greater symmetry, ghost messages are also\r
> > algorithmically simpler.  We continue to support the old user metadata\r
> > format, so this patch can't delete any code, but when we do remove\r
> > support for the old format, several functions can simply be deleted.\r
> >\r
> > [1] id:8738h7kv2q.fsf@qmul.ac.uk\r
> > ---\r
> >  lib/database.cc | 86 +++++++++++++++++++++++++++++++++++++++++++++++++--------\r
> >  1 file changed, 75 insertions(+), 11 deletions(-)\r
> >\r
> > diff --git a/lib/database.cc b/lib/database.cc\r
> > index c641bcd..fdcc526 100644\r
> > --- a/lib/database.cc\r
> > +++ b/lib/database.cc\r
> > @@ -1752,6 +1752,12 @@ _get_metadata_thread_id_key (void *ctx, const char *message_id)\r
> >                         message_id);\r
> >  }\r
> >  \r
> > +static notmuch_status_t\r
> > +_resolve_message_id_to_thread_id_old (notmuch_database_t *notmuch,\r
> > +                                 void *ctx,\r
> > +                                 const char *message_id,\r
> > +                                 const char **thread_id_ret);\r
> > +\r
> >  /* Find the thread ID to which the message with 'message_id' belongs.\r
> >   *\r
> >   * Note: 'thread_id_ret' must not be NULL!\r
> > @@ -1760,9 +1766,9 @@ _get_metadata_thread_id_key (void *ctx, const char *message_id)\r
> >   *\r
> >   * Note: If there is no message in the database with the given\r
> >   * 'message_id' then a new thread_id will be allocated for this\r
> > - * message and stored in the database metadata, (where this same\r
> > + * message ID and stored in the database metadata so that the\r
> >   * thread ID can be looked up if the message is added to the database\r
> > - * later).\r
> > + * later.\r
> >   */\r
> >  static notmuch_status_t\r
> >  _resolve_message_id_to_thread_id (notmuch_database_t *notmuch,\r
> > @@ -1770,6 +1776,49 @@ _resolve_message_id_to_thread_id (notmuch_database_t *notmuch,\r
> >                               const char *message_id,\r
> >                               const char **thread_id_ret)\r
> >  {\r
> > +    notmuch_private_status_t status;\r
> > +    notmuch_message_t *message;\r
> > +\r
> > +    if (! (notmuch->features & NOTMUCH_FEATURE_GHOSTS))\r
> > +   return _resolve_message_id_to_thread_id_old (notmuch, ctx, message_id,\r
> > +                                                thread_id_ret);\r
> > +\r
> > +    /* Look for this message (regular or ghost) */\r
> > +    message = _notmuch_message_create_for_message_id (\r
> > +   notmuch, message_id, &status);\r
> > +    if (status == NOTMUCH_PRIVATE_STATUS_SUCCESS) {\r
> > +   /* Message exists */\r
> > +   *thread_id_ret = talloc_steal (\r
> > +       ctx, notmuch_message_get_thread_id (message));\r
> > +    } else if (status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {\r
> > +   /* Message did not exist.  Give it a fresh thread ID and\r
> > +    * populate this message as a ghost message. */\r
> > +   *thread_id_ret = talloc_strdup (\r
> > +       ctx, _notmuch_database_generate_thread_id (notmuch));\r
> > +   if (! *thread_id_ret) {\r
> > +       status = NOTMUCH_PRIVATE_STATUS_OUT_OF_MEMORY;\r
> > +   } else {\r
> > +       status = _notmuch_message_initialize_ghost (message, *thread_id_ret);\r
> > +       if (status == 0)\r
> > +           /* Commit the new ghost message */\r
> > +           _notmuch_message_sync (message);\r
> > +   }\r
> > +    } else {\r
> > +   /* Create failed. Fall through. */\r
> > +    }\r
> > +\r
> > +    notmuch_message_destroy (message);\r
> > +\r
> > +    return COERCE_STATUS (status, "Error creating ghost message");\r
> > +}\r
> > +\r
> > +/* Pre-ghost messages _resolve_message_id_to_thread_id */\r
> > +static notmuch_status_t\r
> > +_resolve_message_id_to_thread_id_old (notmuch_database_t *notmuch,\r
> > +                                 void *ctx,\r
> > +                                 const char *message_id,\r
> > +                                 const char **thread_id_ret)\r
> > +{\r
> >      notmuch_status_t status;\r
> >      notmuch_message_t *message;\r
> >      string thread_id_string;\r
> > @@ -2007,7 +2056,7 @@ _consume_metadata_thread_id (void *ctx, notmuch_database_t *notmuch,\r
> >      }\r
> >  }\r
> >  \r
> > -/* Given a (mostly empty) 'message' and its corresponding\r
> > +/* Given a blank or ghost 'message' and its corresponding\r
> >   * 'message_file' link it to existing threads in the database.\r
> >   *\r
> >   * The first check is in the metadata of the database to see if we\r
> \r
> There is quite a lot of comment below this and it is not clear to me how\r
> much of it applies to the is_ghost case. In particular does the \r
> \r
>   * Finally, we look in the database for existing message that\r
>   * reference 'message'.\r
> \r
> still apply? I would have thought that we would already have done that\r
> in the is_ghost case. \r
\r
Good point.  The comment isn't really wrong, but it isn't right\r
either.  How about\r
\r
diff --git a/lib/database.cc b/lib/database.cc\r
index 6e51a72..d063ec8 100644\r
--- a/lib/database.cc\r
+++ b/lib/database.cc\r
@@ -2121,10 +2121,13 @@ _consume_metadata_thread_id (void *ctx, notmuch_database_t *notmuch,\r
 /* Given a blank or ghost 'message' and its corresponding\r
  * 'message_file' link it to existing threads in the database.\r
  *\r
- * The first check is in the metadata of the database to see if we\r
- * have pre-allocated a thread_id in advance for this message, (which\r
- * would have happened if a message was previously added that\r
- * referenced this one).\r
+ * First, if is_ghost, this retrieves the thread ID already stored in\r
+ * the message (which will be the case if a message was previously\r
+ * added that referenced this one).  If the message is blank\r
+ * (!is_ghost), it doesn't have a thread ID yet (we'll generate one\r
+ * later in this function).  If the database does not support ghost\r
+ * messages, this checks for a thread ID stored in database metadata\r
+ * for this message ID.\r
  *\r
  * Second, we look at 'message_file' and its link-relevant headers\r
  * (References and In-Reply-To) for message IDs.\r
@@ -2132,7 +2135,7 @@ _consume_metadata_thread_id (void *ctx, notmuch_database_t *notmuch,\r
  * Finally, we look in the database for existing message that\r
  * reference 'message'.\r
  *\r
- * In all cases, we assign to the current message the first thread_id\r
+ * In all cases, we assign to the current message the first thread ID\r
  * found (through either parent or child). We will also merge any\r
  * existing, distinct threads where this message belongs to both,\r
  * (which is not uncommon when messages are processed out of order).\r
\r
?\r
\r
> Of course this also applies to the actual code which seems to call\r
> _notmuch_database_link_message_to_children unconditionally. It might be\r
> worth a comment in any case.\r
> \r
> Best wishes\r
> \r
> Mark\r
> \r
> \r
> > @@ -2035,16 +2084,22 @@ _consume_metadata_thread_id (void *ctx, notmuch_database_t *notmuch,\r
> >  static notmuch_status_t\r
> >  _notmuch_database_link_message (notmuch_database_t *notmuch,\r
> >                             notmuch_message_t *message,\r
> > -                           notmuch_message_file_t *message_file)\r
> > +                           notmuch_message_file_t *message_file,\r
> > +                           notmuch_bool_t is_ghost)\r
> >  {\r
> >      void *local = talloc_new (NULL);\r
> >      notmuch_status_t status;\r
> > -    const char *thread_id;\r
> > +    const char *thread_id = NULL;\r
> >  \r
> >      /* Check if the message already had a thread ID */\r
> > -    thread_id = _consume_metadata_thread_id (local, notmuch, message);\r
> > -    if (thread_id)\r
> > -   _notmuch_message_add_term (message, "thread", thread_id);\r
> > +    if (notmuch->features & NOTMUCH_FEATURE_GHOSTS) {\r
> > +   if (is_ghost)\r
> > +       thread_id = notmuch_message_get_thread_id (message);\r
> > +    } else {\r
> > +   thread_id = _consume_metadata_thread_id (local, notmuch, message);\r
> > +   if (thread_id)\r
> > +       _notmuch_message_add_term (message, "thread", thread_id);\r
> > +    }\r
> >  \r
> >      status = _notmuch_database_link_message_to_parents (notmuch, message,\r
> >                                                     message_file,\r
> > @@ -2079,6 +2134,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,\r
> >      notmuch_message_t *message = NULL;\r
> >      notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS, ret2;\r
> >      notmuch_private_status_t private_status;\r
> > +    notmuch_bool_t is_ghost = false;\r
> >  \r
> >      const char *date, *header;\r
> >      const char *from, *to, *subject;\r
> > @@ -2171,12 +2227,20 @@ notmuch_database_add_message (notmuch_database_t *notmuch,\r
> >  \r
> >     _notmuch_message_add_filename (message, filename);\r
> >  \r
> > -   /* Is this a newly created message object? */\r
> > -   if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {\r
> > +   /* Is this a newly created message object or a ghost\r
> > +    * message?  We have to be slightly careful: if this is a\r
> > +    * blank message, it's not safe to call\r
> > +    * notmuch_message_get_flag yet. */\r
> > +   if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND ||\r
> > +       (is_ghost = notmuch_message_get_flag (\r
> > +           message, NOTMUCH_MESSAGE_FLAG_GHOST))) {\r
> >         _notmuch_message_add_term (message, "type", "mail");\r
> > +       if (is_ghost)\r
> > +           /* Convert ghost message to a regular message */\r
> > +           _notmuch_message_remove_term (message, "type", "ghost");\r
> >  \r
> >         ret = _notmuch_database_link_message (notmuch, message,\r
> > -                                             message_file);\r
> > +                                             message_file, is_ghost);\r
> >         if (ret)\r
> >             goto DONE;\r
> >  \r
> >\r
> > _______________________________________________\r
> > notmuch mailing list\r
> > notmuch@notmuchmail.org\r
> > http://notmuchmail.org/mailman/listinfo/notmuch\r