[PATCH v4 0/2] emacs: show: redesign unread/read logic

[PATCH v4 0/2] emacs: show: redesign unread/read logic

[Mark Walters]

This is v4 of this set. v3 is at id:1395777793-13297-1-git-send-email-markwalters1009@gmail.com

David (dme) was not keen on the logic in the previous patch so I have
tried to make it rather more customisable and made this version much
closer to the existing logic.

This version marks the current message read if it is open. It doesn't
care how you get there: whether it is notmuch commands n/N/p/P next
message etc, emacs commands like scroll up, mouse clicks etc.

The only proviso is it will only mark a message read once (in a single
buffer and between refreshes) as otherwise it is impossible for a user
to choose to mark a message unread.

I, personally, like my previous logic much more. But with this setup
that can be customized easily in my .emacs (we may choose to add other
options into mainline later). Also I think this series fixes all of
the problems with the current read/unread logic mentioned in
id:87a9atmpkf.fsf@qmul.ac.uk are fixed.

The first patch is unchanged. Since the new mark-read function is
small it folded neatly into the patch removing the old unread logic.

Finally, as with previous versions, all tests pass.

Best wishes

Mark




Mark Walters (2):
  test: make test_emacs call post-command-hook
  emacs: show: add an update seen function to post-command-hook

 emacs/notmuch-show.el |   43 ++++++++++++++++++++++++++++---------------
 test/test-lib.el      |   11 +++++++++++
 test/test-lib.sh      |    2 +-
 3 files changed, 40 insertions(+), 16 deletions(-)

-- 
1.7.10.4

[PATCH v4 1/2] test: make test_emacs call post-command-hook

[Mark Walters]

From: David Bremner <david@tethera.net>

The unread/read changes will use the post-command-hook. test_emacs
does not call the post-command-hook. This adds a notmuch-test-progn
which takes a list of commands as argument and executes them in turn
but runs the post-command-hook after each one.

The caller can batch operations (ie to stop post-command-hook from
being interleaved) by wrapping the batch of operations inside a progn.

We also explicitly run the post-command-hook before getting the output
from a test; this makes sense as this will be a place the user would
be seeing the information.
---
 test/test-lib.el |   11 +++++++++++
 test/test-lib.sh |    2 +-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index 437f83f..36afe63 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -52,11 +52,13 @@ (defun notmuch-test-wait ()
 
 (defun test-output (&optional filename)
   "Save current buffer to file FILENAME.  Default FILENAME is OUTPUT."
+  (notmuch-post-command)
   (write-region (point-min) (point-max) (or filename "OUTPUT")))
 
 (defun test-visible-output (&optional filename)
   "Save visible text in current buffer to file FILENAME.  Default
 FILENAME is OUTPUT."
+  (notmuch-post-command)
   (let ((text (visible-buffer-string)))
     (with-temp-file (or filename "OUTPUT") (insert text))))
 
@@ -166,6 +168,15 @@ (defun notmuch-test-expect-equal (output expected)
      (t
       (notmuch-test-report-unexpected output expected)))))
 
+(defun notmuch-post-command ()
+  (run-hooks 'post-command-hook))
+
+(defmacro notmuch-test-progn (&rest body)
+  (cons 'progn
+	(mapcar
+	 (lambda (x) `(prog1 ,x (notmuch-post-command)))
+	 body)))
+
 ;; For historical reasons, we hide deleted tags by default in the test
 ;; suite
 (setq notmuch-tag-deleted-formats
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 8697d6a..e6403e5 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -1139,7 +1139,7 @@ test_emacs () {
 	rm -f OUTPUT
 	touch OUTPUT
 
-	${TEST_EMACSCLIENT} --socket-name="$EMACS_SERVER" --eval "(progn $@)"
+	${TEST_EMACSCLIENT} --socket-name="$EMACS_SERVER" --eval "(notmuch-test-progn $@)"
 }
 
 test_python() {
-- 
1.7.10.4

[PATCH v4 2/2] emacs: show: add an update seen function to post-command-hook

[Mark Walters]

Add a function for updating seen messages to the
post-command-hook. This function calls a customizable (by eg
defcustom) function with parameters the start and end of the current
window and that function can decide what to mark read based on that
and the current point.

Since this is in the post-command-hook it should get called after most
user actions (exceptions include user resizing the window) so it
should be possible to make sure the seen status gets updated whether
the user uses notmuch commands like next-message or normal emacs
commands like scroll-up.

It removes all of the old mark read/seen points but introduces a
simple example function that just marks the current message read if it
is open. This function has one small subtlety: it makes sure it
doesn't mark the same message read twice (in the same instance of the
same buffer); otherwise the post-command-hook makes it impossible for
a user to manually mark a message unread.

This fixes the current bugs (imo) that closed messages can be marked
read, and that opening a closed message does not mark it read.

Another advantage of using the post-command-hook any programmatic use
with point passing through a message will not mark it read.
---
 emacs/notmuch-show.el |   43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 10fc872..2620f84 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -222,6 +222,10 @@ (defcustom notmuch-show-mark-read-tags '("-unread")
   :type '(repeat string)
   :group 'notmuch-show)
 
+(defcustom notmuch-show-mark-read-function #'notmuch-show-seen-current-message
+  "Function to control which messages are marked read."
+  :type 'function
+  :group 'notmuch-show)
 
 (defmacro with-current-notmuch-show-message (&rest body)
   "Evaluate body with current buffer set to the text of current message"
@@ -1156,6 +1160,8 @@ (defun notmuch-show-build-buffer ()
   (let ((inhibit-read-only t))
 
     (notmuch-show-mode)
+    (add-hook 'post-command-hook #'notmuch-show-command-hook nil t)
+
     ;; Don't track undo information for this buffer
     (set 'buffer-undo-list t)
 
@@ -1544,6 +1550,23 @@ (defun notmuch-show-mark-read (&optional unread)
     (apply 'notmuch-show-tag-message
 	   (notmuch-tag-change-list notmuch-show-mark-read-tags unread))))
 
+(defun notmuch-show-seen-current-message (start end)
+  "Mark the current message read if it is open.
+
+We only mark it read once: if it is changed back then that is a
+user decision and we should not override it."
+  (when (and (notmuch-show-message-visible-p)
+	     (not (notmuch-show-get-prop :seen)))
+	(notmuch-show-mark-read)
+	(notmuch-show-set-prop :seen t)))
+
+(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)))))
+
 ;; Functions for getting attributes of several messages in the current
 ;; thread.
 
@@ -1679,9 +1702,7 @@ (defun notmuch-show-next-message (&optional pop-at-end)
 thread, navigate to the next thread in the parent search buffer."
   (interactive "P")
   (if (notmuch-show-goto-message-next)
-      (progn
-	(notmuch-show-mark-read)
-	(notmuch-show-message-adjust))
+      (notmuch-show-message-adjust)
     (if pop-at-end
 	(notmuch-show-next-thread)
       (goto-char (point-max)))))
@@ -1692,7 +1713,6 @@ (defun notmuch-show-previous-message ()
   (if (= (point) (notmuch-show-message-top))
       (notmuch-show-goto-message-previous)
     (notmuch-show-move-to-message-top))
-  (notmuch-show-mark-read)
   (notmuch-show-message-adjust))
 
 (defun notmuch-show-next-open-message (&optional pop-at-end)
@@ -1707,9 +1727,7 @@ (defun notmuch-show-next-open-message (&optional pop-at-end)
     (while (and (setq r (notmuch-show-goto-message-next))
 		(not (notmuch-show-message-visible-p))))
     (if r
-	(progn
-	  (notmuch-show-mark-read)
-	  (notmuch-show-message-adjust))
+	(notmuch-show-message-adjust)
       (if pop-at-end
 	  (notmuch-show-next-thread)
 	(goto-char (point-max))))
@@ -1722,9 +1740,7 @@ (defun notmuch-show-next-matching-message ()
     (while (and (setq r (notmuch-show-goto-message-next))
 		(not (notmuch-show-get-prop :match))))
     (if r
-	(progn
-	  (notmuch-show-mark-read)
-	  (notmuch-show-message-adjust))
+	(notmuch-show-message-adjust)
       (goto-char (point-max)))))
 
 (defun notmuch-show-open-if-matched ()
@@ -1735,8 +1751,7 @@ (defun notmuch-show-open-if-matched ()
 (defun notmuch-show-goto-first-wanted-message ()
   "Move to the first open message and mark it read"
   (goto-char (point-min))
-  (if (notmuch-show-message-visible-p)
-      (notmuch-show-mark-read)
+  (unless (notmuch-show-message-visible-p)
     (notmuch-show-next-open-message))
   (when (eobp)
     ;; There are no matched non-excluded messages so open all matched
@@ -1744,8 +1759,7 @@ (defun notmuch-show-goto-first-wanted-message ()
     (notmuch-show-mapc 'notmuch-show-open-if-matched)
     (force-window-update)
     (goto-char (point-min))
-    (if (notmuch-show-message-visible-p)
-	(notmuch-show-mark-read)
+    (unless (notmuch-show-message-visible-p)
       (notmuch-show-next-open-message))))
 
 (defun notmuch-show-previous-open-message ()
@@ -1755,7 +1769,6 @@ (defun notmuch-show-previous-open-message ()
 		  (notmuch-show-goto-message-previous)
 		(notmuch-show-move-to-message-top))
 	      (not (notmuch-show-message-visible-p))))
-  (notmuch-show-mark-read)
   (notmuch-show-message-adjust))
 
 (defun notmuch-show-view-raw-message ()
-- 
1.7.10.4

Re: [PATCH v4 0/2] emacs: show: redesign unread/read logic

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 1213 bytes --]

On Fri, May 09 2014, Mark Walters wrote:
> This is v4 of this set. v3 is at id:1395777793-13297-1-git-send-email-markwalters1009@gmail.com
>
> David (dme) was not keen on the logic in the previous patch so I have
> tried to make it rather more customisable and made this version much
> closer to the existing logic.
>
> This version marks the current message read if it is open. It doesn't
> care how you get there: whether it is notmuch commands n/N/p/P next
> message etc, emacs commands like scroll up, mouse clicks etc.
>
> The only proviso is it will only mark a message read once (in a single
> buffer and between refreshes) as otherwise it is impossible for a user
> to choose to mark a message unread.

This version looks good to me, thanks for considering the feedback.

> I, personally, like my previous logic much more. But with this setup
> that can be customized easily in my .emacs (we may choose to add other
> options into mainline later). Also I think this series fixes all of
> the problems with the current read/unread logic mentioned in
> id:87a9atmpkf.fsf@qmul.ac.uk are fixed.

Just to confirm: you can get your desired behaviour by writing an
alternative `notmuch-show-mark-read-function'?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 310 bytes --]

Re: [PATCH v4 0/2] emacs: show: redesign unread/read logic

[Mark Walters]

On Fri, 09 May 2014, David Edmondson <dme@dme.org> wrote:
> On Fri, May 09 2014, Mark Walters wrote:
>> This is v4 of this set. v3 is at id:1395777793-13297-1-git-send-email-markwalters1009@gmail.com
>>
>> David (dme) was not keen on the logic in the previous patch so I have
>> tried to make it rather more customisable and made this version much
>> closer to the existing logic.
>>
>> This version marks the current message read if it is open. It doesn't
>> care how you get there: whether it is notmuch commands n/N/p/P next
>> message etc, emacs commands like scroll up, mouse clicks etc.
>>
>> The only proviso is it will only mark a message read once (in a single
>> buffer and between refreshes) as otherwise it is impossible for a user
>> to choose to mark a message unread.
>
> This version looks good to me, thanks for considering the feedback.

Hi

Thanks for looking and testing.

>> I, personally, like my previous logic much more. But with this setup
>> that can be customized easily in my .emacs (we may choose to add other
>> options into mainline later). Also I think this series fixes all of
>> the problems with the current read/unread logic mentioned in
>> id:87a9atmpkf.fsf@qmul.ac.uk are fixed.
>
> Just to confirm: you can get your desired behaviour by writing an
> alternative `notmuch-show-mark-read-function'?

Yes I can confirm that. So if this went in I can get the behaviour of
the earlier series without needing to patch notmuch.

We can decide later if we want to include it (and other possible mark
read functions) as an option, or as a contrib file, or just on the wiki.

Best wishes

Mark

Re: [PATCH v4 0/2] emacs: show: redesign unread/read logic

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 600 bytes --]

On Fri, May 09 2014, Mark Walters wrote:
>> Just to confirm: you can get your desired behaviour by writing an
>> alternative `notmuch-show-mark-read-function'?
>
> Yes I can confirm that. So if this went in I can get the behaviour of
> the earlier series without needing to patch notmuch.
>
> We can decide later if we want to include it (and other possible mark
> read functions) as an option, or as a contrib file, or just on the
> wiki.

I think that it would be convenient to include a function that
implements your preferred behaviour so that others can decide how they
would like it to behave.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 310 bytes --]

[RFC PATCH] emacs: show: mark messages unread if seen in buffer

[Mark Walters]

This adds a function that marks messages unread if they are "seen"
that is a user configurable amount of them has been visible in the
buffer.

To use set notmuch-show-mark-read-function to #'notmuch-show-do-seen
---

This adds the functionality to do my previous mark unread logic (see
id:1395777793-13297-1-git-send-email-markwalters1009@gmail.com) as an
option. It applies on top of the parent series.

This is not intended to be applied as is, but people can see whether
they prefer this or the logic introduced in patch 2. I think we might
as well include the patch 2 logic as an option regardless as some
people will prefer it, and it is very small.

This functionality could go in notmuch-show if many people prefer it,
it could go in contrib if a rather small number like it, or it could
just go on the wiki.

Indeed, a user can have this functionality by loading a file with this
diffs contents (doesn't need to be in the notmuch tree) and setting
notmuch-show-mark-function.

Best wishes

Mark



 emacs/notmuch-show.el |   67 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 2620f84..3f45830 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1550,6 +1550,73 @@ (defun notmuch-show-mark-read (&optional unread)
     (apply 'notmuch-show-tag-message
 	   (notmuch-tag-change-list notmuch-show-mark-read-tags unread))))
 
+(defcustom notmuch-show-seen-lines-needed 0.75
+  "Control which messages get marked seen.
+
+A message is marked seen if both the top of the message and a
+point far \"enough\" down in the message have each been visible
+in the buffer at some point. This parameter controls the
+definition of enough.  Seeing the bottom of message is always
+deemed enough. Additionally, it is deemed enough if a point n
+lines into the message has been visible in the window where n is
+this variable if this variable is an integer and n is this
+variable times the height of the window if this variable is a
+float."
+  :type 'number
+  :group 'notmuch-show)
+
+(defun notmuch-show-update-seen (top-or-bottom)
+  "Update seen status of current message
+
+Mark that we have seen the TOP-OR-BOTTOM of current message."
+  (let ((current (notmuch-show-get-prop :seen)))
+    (unless (or (eq current 'both) (eq current top-or-bottom))
+      (if (not current)
+	  (notmuch-show-set-prop :seen top-or-bottom)
+	(notmuch-show-set-prop :seen 'both)
+	(notmuch-show-mark-read)))))
+
+(defun notmuch-show-do-message-seen (start end)
+  "Update seen status for the current message.
+
+A message is seen if both the top and enough of the rest of the
+message have been visible in the buffer.  Enough means either the
+bottom of the message or a point in the message more than
+LINES-NEEDED lines into the message. LINES-NEEDED is
+`notmuch-show-seen-lines-needed` if that is an integer and that
+times the current window height if it is a float."
+  (let* ((lines-needed (if (integerp notmuch-show-seen-lines-needed)
+			   notmuch-show-seen-lines-needed
+			 (truncate (* notmuch-show-seen-lines-needed (window-body-height)))))
+	 (top (notmuch-show-message-top))
+	 (bottom (notmuch-show-message-bottom)))
+    (when (notmuch-show-message-visible-p)
+      (when (>= top start)
+	(notmuch-show-update-seen 'top))
+      (when (or (<= bottom end)
+		(> (count-screen-lines top end) lines-needed))
+	(notmuch-show-update-seen 'bottom)))))
+
+(defun notmuch-show-do-seen (start end)
+  "Update seen status for all messages between start and end.
+
+We mark the top (bottom) of a message seen if the top (enough of
+the rest of the message) respectively have been visible in the
+buffer. See `notmuch-show-do-message-seen` for the definition of
+enough. When both the top and bottom have been seen we mark the
+message read."
+  (save-excursion
+    (goto-char start)
+    (notmuch-show-do-message-seen start end)
+    (while (and (< (notmuch-show-message-bottom) end)
+		(notmuch-show-goto-message-next))
+      (notmuch-show-do-message-seen start end))
+    ;; This is a work around because emacs gives weird answers for
+    ;; window-end if the buffer ends with invisible text.
+    (when (and (pos-visible-in-window-p (point-max))
+	       (notmuch-show-message-visible-p))
+      (notmuch-show-update-seen 'bottom))))
+
 (defun notmuch-show-seen-current-message (start end)
   "Mark the current message read if it is open.
 
-- 
1.7.10.4

Re: [RFC PATCH] emacs: show: mark messages unread if seen in buffer

[David Edmondson]

On Fri, May 09 2014, Mark Walters wrote:
> To use set notmuch-show-mark-read-function to #'notmuch-show-do-seen

I haven't test this function, but I'd expect it to be an option when
manipulating `notmuch-show-mark-read-function' using custom.

Re: [PATCH v4 1/2] test: make test_emacs call post-command-hook

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> From: David Bremner <david@tethera.net>
>
> The unread/read changes will use the post-command-hook. test_emacs
> does not call the post-command-hook. This adds a notmuch-test-progn
> which takes a list of commands as argument and executes them in turn
> but runs the post-command-hook after each one.

series pushed

d