From: Tomi Ollila <tomi.ollila@iki.fi>
To: Mark Walters <markwalters1009@gmail.com>, 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)
Date: Fri, 10 Jun 2016 18:11:37 +0300
Message-ID: <m2y46djcye.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Fri, Jun 10 2016, Mark Walters <markwalters1009@gmail.com> 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.