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

[notmuch-archives.git] / df / 3ef580593652b2e9b121f880fc9f360cd84a12

Return-Path: <tomi.ollila@iki.fi>\r
X-Original-To: notmuch@notmuchmail.org\r
Delivered-To: notmuch@notmuchmail.org\r
Received: from localhost (localhost [127.0.0.1])\r
 by arlo.cworth.org (Postfix) with ESMTP id 90E146DE034D\r
 for <notmuch@notmuchmail.org>; Mon,  4 Apr 2016 23:53:31 -0700 (PDT)\r
X-Virus-Scanned: Debian amavisd-new at cworth.org\r
X-Spam-Flag: NO\r
X-Spam-Score: 0.625\r
X-Spam-Level: \r
X-Spam-Status: No, score=0.625 tagged_above=-999 required=5 tests=[AWL=-0.027,\r
  SPF_NEUTRAL=0.652] autolearn=disabled\r
Received: from arlo.cworth.org ([127.0.0.1])\r
 by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024)\r
 with ESMTP id Kw2KJrRbRnui for <notmuch@notmuchmail.org>;\r
 Mon,  4 Apr 2016 23:53:21 -0700 (PDT)\r
Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])\r
 by arlo.cworth.org (Postfix) with ESMTP id 9E2F56DE02C2\r
 for <notmuch@notmuchmail.org>; Mon,  4 Apr 2016 23:53:20 -0700 (PDT)\r
Received: from guru.guru-group.fi (localhost [IPv6:::1])\r
 by guru.guru-group.fi (Postfix) with ESMTP id ED20F1000E0;\r
 Tue,  5 Apr 2016 09:53:34 +0300 (EEST)\r
From: Tomi Ollila <tomi.ollila@iki.fi>\r
To: Daniel Kahn Gillmor <dkg@fifthhorseman.net>,\r
 Notmuch Mail <notmuch@notmuchmail.org>\r
Subject: Re: [PATCH v2 3/7] fix thread breakage via ghost-on-removal\r
In-Reply-To: <1459606541-23889-3-git-send-email-dkg@fifthhorseman.net>\r
References: <1459445693-3900-1-git-send-email-dkg@fifthhorseman.net>\r
 <1459606541-23889-1-git-send-email-dkg@fifthhorseman.net>\r
 <1459606541-23889-3-git-send-email-dkg@fifthhorseman.net>\r
User-Agent: Notmuch/0.21+99~g7cbc880 (http://notmuchmail.org) Emacs/24.3.1\r
 (x86_64-unknown-linux-gnu)\r
X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL\r
 $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F\r
 !)g;OY^,BjTbr)Np:%c_o'jj,Z\r
Date: Tue, 05 Apr 2016 09:53:34 +0300\r
Message-ID: <m2lh4sczrl.fsf@guru.guru-group.fi>\r
MIME-Version: 1.0\r
Content-Type: text/plain\r
X-BeenThere: notmuch@notmuchmail.org\r
X-Mailman-Version: 2.1.20\r
Precedence: list\r
List-Id: "Use and development of the notmuch mail system."\r
 <notmuch.notmuchmail.org>\r
List-Unsubscribe: <https://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: <https://notmuchmail.org/mailman/listinfo/notmuch>,\r
 <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
X-List-Received-Date: Tue, 05 Apr 2016 06:53:31 -0000\r
\r
On Sat, Apr 02 2016, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:\r
\r
> ghost-on-removal the solution to T590-thread-breakage.sh that just\r
> adds a ghost message after removing each message.\r
>\r
> It leaks information about whether we've ever seen a given message id,\r
> but it's a fairly simple implementation.\r
>\r
> Note that _resolve_message_id_to_thread_id also introduces new\r
> message_ids to the database, so i think just searching for a given\r
> message ID may introduce the same metadata leakage.\r
>\r
> This differs from v1 of this changeset in that we implement the change\r
> in _notmuch_message_delete, a more "internal" function.\r
\r
I fetched your changes from lair.fifthhorseman.net yesterday and diffed\r
against master; looks pretty good, some quick comments (this email has\r
most relevant changes related to those changes):\r
\r
> ---\r
>  lib/database.cc |  2 +-\r
>  lib/message.cc  | 29 ++++++++++++++++++++++++++---\r
>  2 files changed, 27 insertions(+), 4 deletions(-)\r
>\r
> diff --git a/lib/database.cc b/lib/database.cc\r
> index 3b342f1..d5733c9 100644\r
> --- a/lib/database.cc\r
> +++ b/lib/database.cc\r
> @@ -2557,7 +2557,7 @@ notmuch_database_remove_message (notmuch_database_t *notmuch,\r
>  \r
>      if (status == NOTMUCH_STATUS_SUCCESS && message) {\r
>           status = _notmuch_message_remove_filename (message, filename);\r
> -         if (status == NOTMUCH_STATUS_SUCCESS)\r
> +         if (status == NOTMUCH_STATUS_SUCCESS) \r
\r
It looks to be that this change is insignificant enough so it could be\r
dropped ;)\r
\r
>               _notmuch_message_delete (message);\r
>           else if (status == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID)\r
>               _notmuch_message_sync (message);\r
> diff --git a/lib/message.cc b/lib/message.cc\r
> index 8d72ea2..e414e9c 100644\r
> --- a/lib/message.cc\r
> +++ b/lib/message.cc\r
> @@ -1037,20 +1037,43 @@ _notmuch_message_sync (notmuch_message_t *message)\r
>      message->modified = FALSE;\r
>  }\r
>  \r
> -/* Delete a message document from the database. */\r
> +/* Delete a message document from the database, leaving a ghost\r
> + * message in its place */\r
\r
This comment could tell the condition when ghost message is left --\r
versus the case all ghost messages are dropped (thread becomes empty of\r
mail messages).\r
\r
>  notmuch_status_t\r
>  _notmuch_message_delete (notmuch_message_t *message)\r
>  {\r
>      notmuch_status_t status;\r
>      Xapian::WritableDatabase *db;\r
> +    const char *mid, *tid;\r
> +    notmuch_message_t *ghost;\r
> +    notmuch_private_status_t private_status;\r
> +    notmuch_database_t *notmuch;\r
> +         \r
> +    mid = notmuch_message_get_message_id (message);\r
> +    tid = notmuch_message_get_thread_id (message);\r
> +    notmuch = message->notmuch;\r
>  \r
>      status = _notmuch_database_ensure_writable (message->notmuch);\r
>      if (status)\r
>       return status;\r
>  \r
> -    db = static_cast <Xapian::WritableDatabase *> (message->notmuch->xapian_db);\r
> +    db = static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db);\r
>      db->delete_document (message->doc_id);\r
> -    return NOTMUCH_STATUS_SUCCESS;\r
> +         \r
> +    /* and reintroduce a ghost in its place */\r
> +    ghost = _notmuch_message_create_for_message_id (notmuch, mid, &private_status);\r
\r
In next lines the expected case NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND\r
could be first. Although the performance difference is negligible to me\r
it looks silly do this first check and basically always fail there and\r
then do 'else if' which is likely to succeeed...\r
(your latest version in lair does not return in this first case but sets\r
status to NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID. Perhaps later messages in this\r
thread does the same but those are somewhat out-of-context for this reply).\r
\r
> +    if (private_status == NOTMUCH_PRIVATE_STATUS_SUCCESS) {\r
> +     /* this is deeply weird, and we should not have gotten into\r
> +        this state.  is there a better error message to return\r
> +        here? */\r
> +     return NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;\r
> +    } else if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {\r
> +     private_status = _notmuch_message_initialize_ghost (ghost, tid);\r
> +     if (! private_status)\r
> +         _notmuch_message_sync (ghost);\r
> +    }\r
> +    notmuch_message_destroy (ghost);\r
> +    return COERCE_STATUS (private_status, "Error converting to ghost message");\r
>  }\r
\r
Outside of this patch, but in some of the next messages, adds functions\r
_notmuch_message_has_term() and _notmuch_message_has_term_st(). Perhaps\r
the _notmuch_message_has_term() could be left unimplemented?\r
\r
>  \r
>  /* Transform a blank message into a ghost message.  The caller must\r
> -- \r
> 2.8.0.rc3\r
>\r
> _______________________________________________\r
> notmuch mailing list\r
> notmuch@notmuchmail.org\r
> https://notmuchmail.org/mailman/listinfo/notmuch\r