From c2588c2a59c30f102a827c2b2922b4c420e0cde3 Mon Sep 17 00:00:00 2001 From: Mark Walters Date: Fri, 10 Jun 2016 16:44:46 +0100 Subject: [PATCH] Re: [PATCH v3] emacs: show: improve handling of mark read tagging errors --- 28/26fda9e395a005af06c327f4d4cf814feda10e | 222 ++++++++++++++++++++++ 1 file changed, 222 insertions(+) create mode 100644 28/26fda9e395a005af06c327f4d4cf814feda10e diff --git a/28/26fda9e395a005af06c327f4d4cf814feda10e b/28/26fda9e395a005af06c327f4d4cf814feda10e new file mode 100644 index 000000000..6b11da676 --- /dev/null +++ b/28/26fda9e395a005af06c327f4d4cf814feda10e @@ -0,0 +1,222 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by arlo.cworth.org (Postfix) with ESMTP id 345B06DE028C + for ; Fri, 10 Jun 2016 08:44:59 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at cworth.org +X-Spam-Flag: NO +X-Spam-Score: -0.334 +X-Spam-Level: +X-Spam-Status: No, score=-0.334 tagged_above=-999 required=5 tests=[AWL=0.236, + DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, + FREEMAIL_ENVFROM_END_DIGIT=0.25, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7, + RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_PASS=-0.001] + autolearn=disabled +Received: from arlo.cworth.org ([127.0.0.1]) + by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id ejUJwzjlTwFP for ; + Fri, 10 Jun 2016 08:44:50 -0700 (PDT) +Received: from mail-wm0-f45.google.com (mail-wm0-f45.google.com + [74.125.82.45]) by arlo.cworth.org (Postfix) with ESMTPS id 262C96DE00DF for + ; Fri, 10 Jun 2016 08:44:50 -0700 (PDT) +Received: by mail-wm0-f45.google.com with SMTP id v199so154108272wmv.0 + for ; Fri, 10 Jun 2016 08:44:50 -0700 (PDT) +DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; + h=from:to:subject:in-reply-to:references:user-agent:date:message-id + :mime-version; bh=VWS6cl4rLf/u1mExREHLMwBtqiF496DThhWQrO3Xa38=; + b=qzCJCiqZtcUS0q3PeC2dJYtMDPlsRxL7xPXxClZFoGo/z20wzc2mC8ByDeaX9lmsJO + TzdD+UfgVrjs14LV1dZ2/qe1rhLMr8IOFA/sP9G0oIbkg3OQ0CGAaWa5SYp4AxJdgDMA + zwoWcXFI7SYQCCDmVm/tneDp11DrgW0VkAhnSpWOtpGz+M22GOyTAFOwB+2zfUJ+mbK7 + 4iOTIsUN8tiA+0rkiEzcqW9DkyjkZo5qDXgTbJeRkOhDIaN9j28LOW2AqBpYuzc0a85D + SGUraYxiPK/NWLxXoiePeQxs6b96kSIgDh/CLpesivjLVQH9bDBWBffDLUByPaCJ0eJg + YdUw== +X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; + d=1e100.net; s=20130820; + h=x-gm-message-state:from:to:subject:in-reply-to:references + :user-agent:date:message-id:mime-version; + bh=VWS6cl4rLf/u1mExREHLMwBtqiF496DThhWQrO3Xa38=; + b=LdPeX8ABaNCcFGXWHfKEhYrZp2m1zVDcTnZ9mR1jAY/XwgNXEvYAisJ9oFuquKbnol + 2JFGREmVlsdXGPBGkHXIScUIKtuVkupAPiYvKeuLWDIbTw4XF/vsp/zWqfWngEMsD1OG + R/iN6aAdPK9IQhwmWSKTyyBsQtp+M8su+vj83QEvWy4O1qQz4Z6JP9HSvfkd0dXG/IAv + OGK96SX2avqmKxwyloSzw+eOA+NKSNRyrcShlHvKpspycyY5vJ2wGn4OhdQL+kjcwWRC + zt2KBpH68Y5tCX3mIQ5gOTjGlHPOTmjiLoYzT8CDneEabVVNTNLlcNOJ58mAeytZVraA + TCkg== +X-Gm-Message-State: + ALyK8tJohcCJQy3UVHXqQSHAOKCjUFfpZCs6GmqZlCUYqwqq+WBdMBhb8o4JkL6k6uIBUw== +X-Received: by 10.194.99.244 with SMTP id et20mr3057908wjb.92.1465573488182; + Fri, 10 Jun 2016 08:44:48 -0700 (PDT) +Received: from localhost (5751dfa2.skybroadband.com. [87.81.223.162]) + by smtp.gmail.com with ESMTPSA id e5sm12969957wjj.10.2016.06.10.08.44.47 + (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); + Fri, 10 Jun 2016 08:44:47 -0700 (PDT) +From: Mark Walters +To: Tomi Ollila , notmuch@notmuchmail.org +Subject: Re: [PATCH v3] emacs: show: improve handling of mark read tagging + errors +In-Reply-To: +References: <1465466050-27220-1-git-send-email-markwalters1009@gmail.com> + <1465553965-3260-1-git-send-email-markwalters1009@gmail.com> + +User-Agent: Notmuch/0.22+30~g42683ad (http://notmuchmail.org) Emacs/24.4.1 + (x86_64-pc-linux-gnu) +Date: Fri, 10 Jun 2016 16:44:46 +0100 +Message-ID: <87inxhgia9.fsf@qmul.ac.uk> +MIME-Version: 1.0 +Content-Type: text/plain +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.20 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Fri, 10 Jun 2016 15:44:59 -0000 + + +On Fri, 10 Jun 2016, Tomi Ollila wrote: +> On Fri, Jun 10 2016, Mark Walters wrote: +> +>> Previously if a marking read tag change (i.e., removing the unread +>> tag) failed for some reason, such as a locked database, then no more +>> mark read tag changes would be attempted in that buffer. +>> +>> This handles the error more gracefully. There is not much we can do +>> yet about dealing with the error itself, and marking read is probably +>> not important enough to warrant keeping a queue of pending changes or +>> anything. +>> +>> However this commit changes it so that +>> +>> - we do try and make future mark read tag changes. +>> +>> - we display the tag state correctly: i.e. we don't display the tag as +>> deleted (no strike through) +>> +>> - and since we know the tag change failed we can try to mark this +>> message read in the future. Indeed, since the code uses the +>> post-command hook we will try again on the next keypress (unless the +>> user has left the message). +>> +>> We indicate to the user that these mark read tag changes may have +>> failed in the header-line and by a message in the echo area. +>> --- +>> +>> Hi +>> +>> The best level of user notification in case of an error is +>> unclear. The best we came up with on irc is this one: +>> +>> On first error, the headerline is changed to say (in warning face) +>> that some mark read tag changes may have failed. +>> +>> On each error, which will occur on each call to +>> notmuch-show-command-hook (so roughly after each keypress) we write +>> the error to the error buffer and we send a message to the echo area. +>> +>> In principle I would like to send a single message to the echo area +>> and have it persist for a few seconds. However, the echo area is +>> cleared after each keypress so this seems difficult. Moreover, this +>> clearing means if we send the message a single time and the user +>> enters the message with repeated cursor-downs then the message will +>> disappear as soon as it is displayed. +>> +>> In the future we might want to modify the error code to be something like the +>> message buffer and say "last error repeated x times", but that can +>> come later. +>> +>> Best wishes +>> +>> Mark +>> +>> +>> emacs/notmuch-show.el | 17 ++++++++++++++++- +>> 1 file changed, 16 insertions(+), 1 deletion(-) +>> +>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el +>> index fea39fa..406f418 100644 +>> --- a/emacs/notmuch-show.el +>> +++ b/emacs/notmuch-show.el +>> @@ -1701,12 +1701,27 @@ user decision and we should not override it." +>> (notmuch-show-mark-read) +>> (notmuch-show-set-prop :seen t))) +>> +>> +(defvar notmuch-show--seen-has-errored nil) +>> +(make-variable-buffer-local 'notmuch-show--seen-has-errored) +>> + +>> (defun notmuch-show-command-hook () +>> (when (eq major-mode 'notmuch-show-mode) +>> ;; We need to redisplay to get window-start and window-end correct. +>> (redisplay) +>> (save-excursion +>> - (funcall notmuch-show-mark-read-function (window-start) (window-end))))) +>> + (condition-case nil +>> + (funcall notmuch-show-mark-read-function (window-start) (window-end)) +>> + ((debug error) +>> + ;; The call chain from notmuch-show-mark-read-function writes +>> + ;; and error to the error buffer before calling the error, so +>> + ;; we do not need to do that here. Just tell the user. +> +> I had a bit of difficulties to test this since: +> +> notmuch-show-mark-read-function's value is (lambda +> (start end) +> (notmuch-show-do-seen start +> (point) +> 1000000)) +> +> Original value was +> notmuch-show-seen-current-message +> +> (yes, I've heard there if helper called devel/try-emacs-mua but I just +> ignored that knowledge >;) -- actually I put this to my production use, +> replacing my old solution...) +> +> But, If user changes that then it can be expected that errors are handled, too... + +This is why I preferred your approach of putting the condition-case in +the post-command-hook (over my approach of putting it in +notmuch-show-seen-current-message) as it should be robust to +customisation. I hope you meant it was hard to test as you couldn't +cause an error once you were in the buffer (as the whole buffer was +marked unread when you first entered). + + +> I'll keep running this; is there any better way to test this than +> +> (setq notmuch-command "broken") + +Yes. + +notmuch tag --batch + +is great for this. It locks the database for writes until you ctrl-d to +exit. + +Best wishes + +Mark + + +> +> Change looks good... +> +> +> Tomi +> +> +>> + (message "Warning -- marking message read failed.") +>> + (unless notmuch-show--seen-has-errored +>> + (setq notmuch-show--seen-has-errored 't) +>> + (setq header-line-format +>> + (concat header-line-format +>> + (propertize " [some mark read tag changes may have failed]" +>> + 'face font-lock-warning-face))))))))) +>> +>> (defun notmuch-show-filter-thread (query) +>> "Filter or LIMIT the current thread based on a new query string. -- 2.26.2