Re: [PATCH v3] emacs: show: improve handling of mark read tagging errors
authorTomi Ollila <tomi.ollila@iki.fi>
Fri, 10 Jun 2016 15:11:37 +0000 (18:11 +0300)
committerW. Trevor King <wking@tremily.us>
Sat, 20 Aug 2016 23:22:03 +0000 (16:22 -0700)
d7/c0d1e8efd5df85bb92e81756bbd53456a0fb55 [new file with mode: 0644]

diff --git a/d7/c0d1e8efd5df85bb92e81756bbd53456a0fb55 b/d7/c0d1e8efd5df85bb92e81756bbd53456a0fb55
new file mode 100644 (file)
index 0000000..338c548
--- /dev/null
@@ -0,0 +1,172 @@
+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 706096DE028C\r
+ for <notmuch@notmuchmail.org>; Fri, 10 Jun 2016 08:12:00 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at cworth.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: 0.57\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=0.57 tagged_above=-999 required=5 tests=[AWL=-0.082, \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 ZdwZ8WL4suRR for <notmuch@notmuchmail.org>;\r
+ Fri, 10 Jun 2016 08:11:52 -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 AAAEF6DE00DF\r
+ for <notmuch@notmuchmail.org>; Fri, 10 Jun 2016 08:11:51 -0700 (PDT)\r
+Received: from guru.guru-group.fi (localhost [IPv6:::1])\r
+ by guru.guru-group.fi (Postfix) with ESMTP id C6F09100104;\r
+ Fri, 10 Jun 2016 18:11:37 +0300 (EEST)\r
+From: Tomi Ollila <tomi.ollila@iki.fi>\r
+To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH v3] emacs: show: improve handling of mark read tagging\r
+ errors\r
+In-Reply-To: <1465553965-3260-1-git-send-email-markwalters1009@gmail.com>\r
+References: <1465466050-27220-1-git-send-email-markwalters1009@gmail.com>\r
+ <1465553965-3260-1-git-send-email-markwalters1009@gmail.com>\r
+User-Agent: Notmuch/0.22+32~gd4854c5 (http://notmuchmail.org) Emacs/24.5.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: Fri, 10 Jun 2016 18:11:37 +0300\r
+Message-ID: <m2y46djcye.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: Fri, 10 Jun 2016 15:12:00 -0000\r
+\r
+On Fri, Jun 10 2016, Mark Walters <markwalters1009@gmail.com> wrote:\r
+\r
+> Previously if a marking read tag change (i.e., removing the unread\r
+> tag) failed for some reason, such as a locked database, then no more\r
+> mark read tag changes would be attempted in that buffer.\r
+>\r
+> This handles the error more gracefully. There is not much we can do\r
+> yet about dealing with the error itself, and marking read is probably\r
+> not important enough to warrant keeping a queue of pending changes or\r
+> anything.\r
+>\r
+> However this commit changes it so that\r
+>\r
+> - we do try and make future mark read tag changes.\r
+>\r
+> - we display the tag state correctly: i.e. we don't display the tag as\r
+>   deleted (no strike through)\r
+>\r
+> - and since we know the tag change failed we can try to mark this\r
+>   message read in the future. Indeed, since the code uses the\r
+>   post-command hook we will try again on the next keypress (unless the\r
+>   user has left the message).\r
+>\r
+> We indicate to the user that these mark read tag changes may have\r
+> failed in the header-line and by a message in the echo area.\r
+> ---\r
+>\r
+> Hi\r
+>\r
+> The best level of user notification in case of an error is\r
+> unclear. The best we came up with on irc is this one:\r
+>\r
+> On first error, the headerline is changed to say (in warning face)\r
+> that some mark read tag changes may have failed.\r
+>\r
+> On each error, which will occur on each call to\r
+> notmuch-show-command-hook (so roughly after each keypress) we write\r
+> the error to the error buffer and we send a message to the echo area.\r
+>\r
+> In principle I would like to send a single message to the echo area\r
+> and have it persist for a few seconds. However, the echo area is\r
+> cleared after each keypress so this seems difficult. Moreover, this\r
+> clearing means if we send the message a single time and the user\r
+> enters the message with repeated cursor-downs then the message will\r
+> disappear as soon as it is displayed.\r
+>\r
+> In the future we might want to modify the error code to be something like the\r
+> message buffer and say "last error repeated x times", but that can\r
+> come later.\r
+>\r
+> Best wishes\r
+>\r
+> Mark\r
+>\r
+>\r
+>  emacs/notmuch-show.el | 17 ++++++++++++++++-\r
+>  1 file changed, 16 insertions(+), 1 deletion(-)\r
+>\r
+> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el\r
+> index fea39fa..406f418 100644\r
+> --- a/emacs/notmuch-show.el\r
+> +++ b/emacs/notmuch-show.el\r
+> @@ -1701,12 +1701,27 @@ user decision and we should not override it."\r
+>      (notmuch-show-mark-read)\r
+>      (notmuch-show-set-prop :seen t)))\r
+>  \r
+> +(defvar notmuch-show--seen-has-errored nil)\r
+> +(make-variable-buffer-local 'notmuch-show--seen-has-errored)\r
+> +\r
+>  (defun notmuch-show-command-hook ()\r
+>    (when (eq major-mode 'notmuch-show-mode)\r
+>      ;; We need to redisplay to get window-start and window-end correct.\r
+>      (redisplay)\r
+>      (save-excursion\r
+> -      (funcall notmuch-show-mark-read-function (window-start) (window-end)))))\r
+> +      (condition-case nil\r
+> +      (funcall notmuch-show-mark-read-function (window-start) (window-end))\r
+> +    ((debug error)\r
+> +     ;; The call chain from notmuch-show-mark-read-function writes\r
+> +     ;; and error to the error buffer before calling the error, so\r
+> +     ;; we do not need to do that here. Just tell the user.\r
+\r
+I had a bit of difficulties to test this since:\r
+\r
+notmuch-show-mark-read-function's value is (lambda\r
+  (start end)\r
+    (notmuch-show-do-seen start\r
+                          (point)\r
+                          1000000))\r
+\r
+Original value was\r
+notmuch-show-seen-current-message\r
+\r
+(yes, I've heard there if helper called devel/try-emacs-mua but I just\r
+ignored that knowledge >;) -- actually I put this to my production use,\r
+replacing my old solution...)\r
+\r
+But, If user changes that then it can be expected that errors are handled, too...\r
+\r
+\r
+I'll keep running this; is there any better way to test this than\r
+\r
+       (setq notmuch-command "broken")\r
+\r
+Change looks good...\r
+\r
+\r
+Tomi\r
+\r
+\r
+> +     (message "Warning -- marking message read failed.")\r
+> +     (unless notmuch-show--seen-has-errored\r
+> +       (setq notmuch-show--seen-has-errored 't)\r
+> +       (setq header-line-format\r
+> +             (concat header-line-format\r
+> +                     (propertize "  [some mark read tag changes may have failed]"\r
+> +                                 'face font-lock-warning-face)))))))))\r
+>  \r
+>  (defun notmuch-show-filter-thread (query)\r
+>    "Filter or LIMIT the current thread based on a new query string.\r