From 3807e2ebe56ad8c986fac288c657c0192c2a521d Mon Sep 17 00:00:00 2001 From: Tomi Ollila Date: Fri, 10 Jun 2016 18:11:37 +0300 Subject: [PATCH] Re: [PATCH v3] emacs: show: improve handling of mark read tagging errors --- d7/c0d1e8efd5df85bb92e81756bbd53456a0fb55 | 172 ++++++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 d7/c0d1e8efd5df85bb92e81756bbd53456a0fb55 diff --git a/d7/c0d1e8efd5df85bb92e81756bbd53456a0fb55 b/d7/c0d1e8efd5df85bb92e81756bbd53456a0fb55 new file mode 100644 index 000000000..338c54859 --- /dev/null +++ b/d7/c0d1e8efd5df85bb92e81756bbd53456a0fb55 @@ -0,0 +1,172 @@ +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 706096DE028C + for ; Fri, 10 Jun 2016 08:12:00 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at cworth.org +X-Spam-Flag: NO +X-Spam-Score: 0.57 +X-Spam-Level: +X-Spam-Status: No, score=0.57 tagged_above=-999 required=5 tests=[AWL=-0.082, + SPF_NEUTRAL=0.652] 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 ZdwZ8WL4suRR for ; + Fri, 10 Jun 2016 08:11:52 -0700 (PDT) +Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) + by arlo.cworth.org (Postfix) with ESMTP id AAAEF6DE00DF + for ; Fri, 10 Jun 2016 08:11:51 -0700 (PDT) +Received: from guru.guru-group.fi (localhost [IPv6:::1]) + by guru.guru-group.fi (Postfix) with ESMTP id C6F09100104; + Fri, 10 Jun 2016 18:11:37 +0300 (EEST) +From: Tomi Ollila +To: Mark Walters , notmuch@notmuchmail.org +Subject: Re: [PATCH v3] emacs: show: improve handling of mark read tagging + errors +In-Reply-To: <1465553965-3260-1-git-send-email-markwalters1009@gmail.com> +References: <1465466050-27220-1-git-send-email-markwalters1009@gmail.com> + <1465553965-3260-1-git-send-email-markwalters1009@gmail.com> +User-Agent: Notmuch/0.22+32~gd4854c5 (http://notmuchmail.org) Emacs/24.5.1 + (x86_64-unknown-linux-gnu) +X-Face: HhBM'cA~ +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:12:00 -0000 + +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... + + +I'll keep running this; is there any better way to test this than + + (setq notmuch-command "broken") + +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