Re: [PATCH v3] emacs: show: improve handling of mark read tagging errors
authorMark Walters <markwalters1009@gmail.com>
Fri, 10 Jun 2016 15:44:46 +0000 (16:44 +0100)
committerW. Trevor King <wking@tremily.us>
Sat, 20 Aug 2016 23:22:03 +0000 (16:22 -0700)
28/26fda9e395a005af06c327f4d4cf814feda10e [new file with mode: 0644]

diff --git a/28/26fda9e395a005af06c327f4d4cf814feda10e b/28/26fda9e395a005af06c327f4d4cf814feda10e
new file mode 100644 (file)
index 0000000..6b11da6
--- /dev/null
@@ -0,0 +1,222 @@
+Return-Path: <markwalters1009@gmail.com>\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 345B06DE028C\r
+ for <notmuch@notmuchmail.org>; Fri, 10 Jun 2016 08:44:59 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at cworth.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.334\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.334 tagged_above=-999 required=5 tests=[AWL=0.236,\r
+  DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1,\r
+ FREEMAIL_ENVFROM_END_DIGIT=0.25, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7,\r
+ RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_PASS=-0.001]\r
+ 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 ejUJwzjlTwFP for <notmuch@notmuchmail.org>;\r
+ Fri, 10 Jun 2016 08:44:50 -0700 (PDT)\r
+Received: from mail-wm0-f45.google.com (mail-wm0-f45.google.com\r
+ [74.125.82.45]) by arlo.cworth.org (Postfix) with ESMTPS id 262C96DE00DF for\r
+ <notmuch@notmuchmail.org>; Fri, 10 Jun 2016 08:44:50 -0700 (PDT)\r
+Received: by mail-wm0-f45.google.com with SMTP id v199so154108272wmv.0\r
+ for <notmuch@notmuchmail.org>; Fri, 10 Jun 2016 08:44:50 -0700 (PDT)\r
+DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;\r
+ h=from:to:subject:in-reply-to:references:user-agent:date:message-id\r
+ :mime-version; bh=VWS6cl4rLf/u1mExREHLMwBtqiF496DThhWQrO3Xa38=;\r
+ b=qzCJCiqZtcUS0q3PeC2dJYtMDPlsRxL7xPXxClZFoGo/z20wzc2mC8ByDeaX9lmsJO\r
+ TzdD+UfgVrjs14LV1dZ2/qe1rhLMr8IOFA/sP9G0oIbkg3OQ0CGAaWa5SYp4AxJdgDMA\r
+ zwoWcXFI7SYQCCDmVm/tneDp11DrgW0VkAhnSpWOtpGz+M22GOyTAFOwB+2zfUJ+mbK7\r
+ 4iOTIsUN8tiA+0rkiEzcqW9DkyjkZo5qDXgTbJeRkOhDIaN9j28LOW2AqBpYuzc0a85D\r
+ SGUraYxiPK/NWLxXoiePeQxs6b96kSIgDh/CLpesivjLVQH9bDBWBffDLUByPaCJ0eJg\r
+ YdUw==\r
+X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
+ d=1e100.net; s=20130820;\r
+ h=x-gm-message-state:from:to:subject:in-reply-to:references\r
+ :user-agent:date:message-id:mime-version;\r
+ bh=VWS6cl4rLf/u1mExREHLMwBtqiF496DThhWQrO3Xa38=;\r
+ b=LdPeX8ABaNCcFGXWHfKEhYrZp2m1zVDcTnZ9mR1jAY/XwgNXEvYAisJ9oFuquKbnol\r
+ 2JFGREmVlsdXGPBGkHXIScUIKtuVkupAPiYvKeuLWDIbTw4XF/vsp/zWqfWngEMsD1OG\r
+ R/iN6aAdPK9IQhwmWSKTyyBsQtp+M8su+vj83QEvWy4O1qQz4Z6JP9HSvfkd0dXG/IAv\r
+ OGK96SX2avqmKxwyloSzw+eOA+NKSNRyrcShlHvKpspycyY5vJ2wGn4OhdQL+kjcwWRC\r
+ zt2KBpH68Y5tCX3mIQ5gOTjGlHPOTmjiLoYzT8CDneEabVVNTNLlcNOJ58mAeytZVraA\r
+ TCkg==\r
+X-Gm-Message-State:\r
+ ALyK8tJohcCJQy3UVHXqQSHAOKCjUFfpZCs6GmqZlCUYqwqq+WBdMBhb8o4JkL6k6uIBUw==\r
+X-Received: by 10.194.99.244 with SMTP id et20mr3057908wjb.92.1465573488182;\r
+ Fri, 10 Jun 2016 08:44:48 -0700 (PDT)\r
+Received: from localhost (5751dfa2.skybroadband.com. [87.81.223.162])\r
+ by smtp.gmail.com with ESMTPSA id e5sm12969957wjj.10.2016.06.10.08.44.47\r
+ (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\r
+ Fri, 10 Jun 2016 08:44:47 -0700 (PDT)\r
+From: Mark Walters <markwalters1009@gmail.com>\r
+To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH v3] emacs: show: improve handling of mark read tagging\r
+ errors\r
+In-Reply-To: <m2y46djcye.fsf@guru.guru-group.fi>\r
+References: <1465466050-27220-1-git-send-email-markwalters1009@gmail.com>\r
+ <1465553965-3260-1-git-send-email-markwalters1009@gmail.com>\r
+ <m2y46djcye.fsf@guru.guru-group.fi>\r
+User-Agent: Notmuch/0.22+30~g42683ad (http://notmuchmail.org) Emacs/24.4.1\r
+ (x86_64-pc-linux-gnu)\r
+Date: Fri, 10 Jun 2016 16:44:46 +0100\r
+Message-ID: <87inxhgia9.fsf@qmul.ac.uk>\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:44:59 -0000\r
+\r
+\r
+On Fri, 10 Jun 2016, Tomi Ollila <tomi.ollila@iki.fi> wrote:\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
+This is why I preferred your approach of putting the condition-case in\r
+the post-command-hook (over my approach of putting it in\r
+notmuch-show-seen-current-message) as it should be robust to\r
+customisation. I hope you meant it was hard to test as you couldn't\r
+cause an error once you were in the buffer (as the whole buffer was\r
+marked unread when you first entered).\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
+Yes.\r
+\r
+notmuch tag --batch\r
+\r
+is great for this. It locks the database for writes until you ctrl-d to\r
+exit.\r
+\r
+Best wishes\r
+\r
+Mark\r
+\r
+\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