unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 0/3] Make notmuch-show-refresh-view retain state by default
@ 2012-02-19  6:22 Austin Clements
  2012-02-19  6:22 ` [PATCH 1/3] emacs: Fix out-of-date declare-function Austin Clements
                   ` (6 more replies)
  0 siblings, 7 replies; 23+ messages in thread
From: Austin Clements @ 2012-02-19  6:22 UTC (permalink / raw)
  To: notmuch

Based on the thread at id:"20120213152858.GO27039@mit.edu" it seems
like people want show refresh to retain message state by default (I
certainly do), rather than reset it by default.  As a nice bonus, this
fixes a broken test.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/3] emacs: Fix out-of-date declare-function
  2012-02-19  6:22 [PATCH 0/3] Make notmuch-show-refresh-view retain state by default Austin Clements
@ 2012-02-19  6:22 ` Austin Clements
  2012-02-19  6:22 ` [PATCH 2/3] emacs: Reverse the meaning of notmuch-show-refresh-view's argument Austin Clements
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 23+ messages in thread
From: Austin Clements @ 2012-02-19  6:22 UTC (permalink / raw)
  To: notmuch

The names of the arguments to notmuch-show-refresh-view had gotten out
of sync between the declare-function and the real thing.
---
 emacs/notmuch-crypto.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index c7ef1eb..972f26e 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -120,7 +120,7 @@ mode."
      :notmuch-from from)
     (insert "\n")))
 
-(declare-function notmuch-show-refresh-view "notmuch-show" (&optional crypto-switch))
+(declare-function notmuch-show-refresh-view "notmuch-show" (&optional retain-state))
 
 (defun notmuch-crypto-sigstatus-good-callback (button)
   (let* ((sigstatus (button-get button :notmuch-sigstatus))
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/3] emacs: Reverse the meaning of notmuch-show-refresh-view's argument
  2012-02-19  6:22 [PATCH 0/3] Make notmuch-show-refresh-view retain state by default Austin Clements
  2012-02-19  6:22 ` [PATCH 1/3] emacs: Fix out-of-date declare-function Austin Clements
@ 2012-02-19  6:22 ` Austin Clements
  2012-02-19  6:22 ` [PATCH 3/3] News for retaining state when refreshing notmuch show Austin Clements
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 23+ messages in thread
From: Austin Clements @ 2012-02-19  6:22 UTC (permalink / raw)
  To: notmuch

Consensus seems to be that people prefer that refreshing show buffers
retains state by default, rather than resetting it by default.  This
turns out to be the case in the code, as well.  In fact, there's even
a test for this that's been marked broken for several months, which
this patch finally gets to mark as fixed.
---
 emacs/notmuch-crypto.el |    4 ++--
 emacs/notmuch-show.el   |   18 +++++++++---------
 test/emacs              |    1 -
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index 972f26e..94da325 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -120,7 +120,7 @@ mode."
      :notmuch-from from)
     (insert "\n")))
 
-(declare-function notmuch-show-refresh-view "notmuch-show" (&optional retain-state))
+(declare-function notmuch-show-refresh-view "notmuch-show" (&optional reset-state))
 
 (defun notmuch-crypto-sigstatus-good-callback (button)
   (let* ((sigstatus (button-get button :notmuch-sigstatus))
@@ -145,7 +145,7 @@ mode."
 	(insert "\n")
 	(call-process "gpg" nil t t "--list-keys" keyid))
       (recenter -1))
-    (notmuch-show-refresh-view)))
+    (notmuch-show-refresh-view t)))
 
 (defun notmuch-crypto-insert-encstatus-button (encstatus)
   (let* ((status (plist-get encstatus :status))
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index aa9ccee..da1289a 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -957,7 +957,7 @@ current buffer, if possible."
   (message (if notmuch-show-process-crypto
 	       "Processing cryptographic MIME parts."
 	     "Not processing cryptographic MIME parts."))
-  (notmuch-show-refresh-view t))
+  (notmuch-show-refresh-view))
 
 (defun notmuch-show-toggle-elide-non-matching ()
   "Toggle the display of non-matching messages."
@@ -966,7 +966,7 @@ current buffer, if possible."
   (message (if notmuch-show-elide-non-matching-messages
 	       "Showing matching messages only."
 	     "Showing all messages."))
-  (notmuch-show-refresh-view t))
+  (notmuch-show-refresh-view))
 
 (defun notmuch-show-toggle-thread-indentation ()
   "Toggle the indentation of threads."
@@ -975,7 +975,7 @@ current buffer, if possible."
   (message (if notmuch-show-indent-content
 	       "Content is indented."
 	     "Content is not indented."))
-  (notmuch-show-refresh-view t))
+  (notmuch-show-refresh-view))
 
 (defun notmuch-show-insert-tree (tree depth)
   "Insert the message tree TREE at depth DEPTH in the current thread."
@@ -1118,17 +1118,17 @@ This includes:
       (message "Previously current message not found."))
     (notmuch-show-message-adjust)))
 
-(defun notmuch-show-refresh-view (&optional retain-state)
+(defun notmuch-show-refresh-view (&optional reset-state)
   "Refresh the current view.
 
 Refreshes the current view, observing changes in display
-preferences. If RETAIN-STATE is non-nil then the state of the
-buffer is stored and re-applied after the refresh."
+preferences. If invoked with a prefix argument (or RESET-STATE is
+non-nil) then the state of the buffer (open/closed messages) is
+reset based on the original query."
   (interactive "P")
   (let ((inhibit-read-only t)
-	state)
-    (if retain-state
-	(setq state (notmuch-show-capture-state)))
+	(state (unless reset-state
+		 (notmuch-show-capture-state))))
     (erase-buffer)
     (notmuch-show-worker)
     (if state
diff --git a/test/emacs b/test/emacs
index b74cfa9..2dffee8 100755
--- a/test/emacs
+++ b/test/emacs
@@ -456,7 +456,6 @@ test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Refresh modified show buffer"
-test_subtest_known_broken
 test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
 	    (notmuch-show-toggle-message)
 	    (notmuch-show-next-message)
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/3] News for retaining state when refreshing notmuch show
  2012-02-19  6:22 [PATCH 0/3] Make notmuch-show-refresh-view retain state by default Austin Clements
  2012-02-19  6:22 ` [PATCH 1/3] emacs: Fix out-of-date declare-function Austin Clements
  2012-02-19  6:22 ` [PATCH 2/3] emacs: Reverse the meaning of notmuch-show-refresh-view's argument Austin Clements
@ 2012-02-19  6:22 ` Austin Clements
  2012-02-19  9:43 ` [PATCH 0/3] Make notmuch-show-refresh-view retain state by default Tomi Ollila
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 23+ messages in thread
From: Austin Clements @ 2012-02-19  6:22 UTC (permalink / raw)
  To: notmuch

---
 NEWS |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/NEWS b/NEWS
index e9abb86..75fa2f4 100644
--- a/NEWS
+++ b/NEWS
@@ -63,6 +63,11 @@ More flexible and consistent tagging operations
 
     (notmuch-show-tag-message "-unread")
 
+Refreshing the show view ('=' by default) no longer opens or closes messages
+
+  To get the old behavior of putting messages back in their initial
+  opened/closed state, use a prefix argument, e.g., C-u =.
+
 Library changes
 ---------------
 
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 0/3] Make notmuch-show-refresh-view retain state by default
  2012-02-19  6:22 [PATCH 0/3] Make notmuch-show-refresh-view retain state by default Austin Clements
                   ` (2 preceding siblings ...)
  2012-02-19  6:22 ` [PATCH 3/3] News for retaining state when refreshing notmuch show Austin Clements
@ 2012-02-19  9:43 ` Tomi Ollila
  2012-02-19 17:25   ` Austin Clements
  2012-02-19 17:16 ` Dmitry Kurochkin
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Tomi Ollila @ 2012-02-19  9:43 UTC (permalink / raw)
  To: Austin Clements, notmuch

On Sun, 19 Feb 2012 01:22:10 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Based on the thread at id:"20120213152858.GO27039@mit.edu" it seems
> like people want show refresh to retain message state by default (I
> certainly do), rather than reset it by default.  As a nice bonus, this
> fixes a broken test.

Hmm

Every '=' keypress in a thread removes 'unread' tag from next unread 
message :o

That is probably desirable feature when refreshing view without
retaining state. Also, probably notmuch-show-refresh-view should
be fixed not to mark next message unread when retaining state ?

Tomi

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 0/3] Make notmuch-show-refresh-view retain state by default
  2012-02-19  6:22 [PATCH 0/3] Make notmuch-show-refresh-view retain state by default Austin Clements
                   ` (3 preceding siblings ...)
  2012-02-19  9:43 ` [PATCH 0/3] Make notmuch-show-refresh-view retain state by default Tomi Ollila
@ 2012-02-19 17:16 ` Dmitry Kurochkin
  2012-02-19 18:02 ` [PATCH v2 0/4] " Austin Clements
  2012-02-21 15:42 ` [PATCH v3 0/3] Fix refreshing with state and make it the default Austin Clements
  6 siblings, 0 replies; 23+ messages in thread
From: Dmitry Kurochkin @ 2012-02-19 17:16 UTC (permalink / raw)
  To: Austin Clements, notmuch

On Sun, 19 Feb 2012 01:22:10 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Based on the thread at id:"20120213152858.GO27039@mit.edu" it seems
> like people want show refresh to retain message state by default (I
> certainly do), rather than reset it by default.  As a nice bonus, this
> fixes a broken test.
> 

All three patches look good to me.

Regards,
  Dmitry

> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 0/3] Make notmuch-show-refresh-view retain state by default
  2012-02-19  9:43 ` [PATCH 0/3] Make notmuch-show-refresh-view retain state by default Tomi Ollila
@ 2012-02-19 17:25   ` Austin Clements
  2012-02-19 17:29     ` Dmitry Kurochkin
  0 siblings, 1 reply; 23+ messages in thread
From: Austin Clements @ 2012-02-19 17:25 UTC (permalink / raw)
  To: Tomi Ollila; +Cc: notmuch

Quoth Tomi Ollila on Feb 19 at 11:43 am:
> On Sun, 19 Feb 2012 01:22:10 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > Based on the thread at id:"20120213152858.GO27039@mit.edu" it seems
> > like people want show refresh to retain message state by default (I
> > certainly do), rather than reset it by default.  As a nice bonus, this
> > fixes a broken test.
> 
> Hmm
> 
> Every '=' keypress in a thread removes 'unread' tag from next unread 
> message :o

Oh dear.  It's slightly subtler than that, though.  Refreshing will
remove the unread tag from the first message matching the query.  (I
suspect you had tag:unread in your query?)  This is particularly
confusing for state-retaining refresh because that message might not
even be open.

> That is probably desirable feature when refreshing view without
> retaining state. Also, probably notmuch-show-refresh-view should
> be fixed not to mark next message unread when retaining state ?

I would argue that refresh should never have side-effects, so neither
case should mark anything read.  I'll send a v2.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 0/3] Make notmuch-show-refresh-view retain state by default
  2012-02-19 17:25   ` Austin Clements
@ 2012-02-19 17:29     ` Dmitry Kurochkin
  2012-02-19 18:03       ` Austin Clements
  0 siblings, 1 reply; 23+ messages in thread
From: Dmitry Kurochkin @ 2012-02-19 17:29 UTC (permalink / raw)
  To: Austin Clements, Tomi Ollila; +Cc: notmuch

On Sun, 19 Feb 2012 12:25:41 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Tomi Ollila on Feb 19 at 11:43 am:
> > On Sun, 19 Feb 2012 01:22:10 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > > Based on the thread at id:"20120213152858.GO27039@mit.edu" it seems
> > > like people want show refresh to retain message state by default (I
> > > certainly do), rather than reset it by default.  As a nice bonus, this
> > > fixes a broken test.
> > 
> > Hmm
> > 
> > Every '=' keypress in a thread removes 'unread' tag from next unread 
> > message :o
> 
> Oh dear.  It's slightly subtler than that, though.  Refreshing will
> remove the unread tag from the first message matching the query.  (I
> suspect you had tag:unread in your query?)  This is particularly
> confusing for state-retaining refresh because that message might not
> even be open.
> 
> > That is probably desirable feature when refreshing view without
> > retaining state. Also, probably notmuch-show-refresh-view should
> > be fixed not to mark next message unread when retaining state ?
> 
> I would argue that refresh should never have side-effects, so neither
> case should mark anything read.  I'll send a v2.

I agree.  But this looks like a separate issue, no?  So why v2 instead
of a separate patch/series?

Regards,
  Dmitry

> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 0/4] Make notmuch-show-refresh-view retain state by default
  2012-02-19  6:22 [PATCH 0/3] Make notmuch-show-refresh-view retain state by default Austin Clements
                   ` (4 preceding siblings ...)
  2012-02-19 17:16 ` Dmitry Kurochkin
@ 2012-02-19 18:02 ` Austin Clements
  2012-02-19 18:02   ` [PATCH v2 1/4] emacs: Fix out-of-date declare-function Austin Clements
                     ` (4 more replies)
  2012-02-21 15:42 ` [PATCH v3 0/3] Fix refreshing with state and make it the default Austin Clements
  6 siblings, 5 replies; 23+ messages in thread
From: Austin Clements @ 2012-02-19 18:02 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

v2 adds patch 2, which should address Tomi's bug where refreshing
would mark unexpected messages read.  The other three patches have not
changed.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 1/4] emacs: Fix out-of-date declare-function
  2012-02-19 18:02 ` [PATCH v2 0/4] " Austin Clements
@ 2012-02-19 18:02   ` Austin Clements
  2012-02-21  3:09     ` David Bremner
  2012-02-19 18:02   ` [PATCH v2 2/4] emacs: When refreshing a show buffer, only mark read when resetting state Austin Clements
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 23+ messages in thread
From: Austin Clements @ 2012-02-19 18:02 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

The names of the arguments to notmuch-show-refresh-view had gotten out
of sync between the declare-function and the real thing.
---
 emacs/notmuch-crypto.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index c7ef1eb..972f26e 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -120,7 +120,7 @@ mode."
      :notmuch-from from)
     (insert "\n")))
 
-(declare-function notmuch-show-refresh-view "notmuch-show" (&optional crypto-switch))
+(declare-function notmuch-show-refresh-view "notmuch-show" (&optional retain-state))
 
 (defun notmuch-crypto-sigstatus-good-callback (button)
   (let* ((sigstatus (button-get button :notmuch-sigstatus))
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 2/4] emacs: When refreshing a show buffer, only mark read when resetting state
  2012-02-19 18:02 ` [PATCH v2 0/4] " Austin Clements
  2012-02-19 18:02   ` [PATCH v2 1/4] emacs: Fix out-of-date declare-function Austin Clements
@ 2012-02-19 18:02   ` Austin Clements
  2012-02-19 18:02   ` [PATCH v2 3/4] emacs: Reverse the meaning of notmuch-show-refresh-view's argument Austin Clements
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 23+ messages in thread
From: Austin Clements @ 2012-02-19 18:02 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

If we retain state while refreshing a show buffer, it should not mark
any messages read since it's not a navigation operation (it especially
shouldn't mark the first message matching the query read, which is
what it did previously).  If the user or caller requests that refresh
reset the state of the buffer, then we consider that a navigation
operation, so we do mark the message under point after the refresh
read.

This is implemented by moving responsibility for read-marking out of
notmuch-show-worker and into its caller.
---
 emacs/notmuch-show.el |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index aa9ccee..d036c54 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1047,7 +1047,10 @@ function is used."
     (setq notmuch-show-thread-id thread-id
 	  notmuch-show-parent-buffer parent-buffer
 	  notmuch-show-query-context query-context)
-    (notmuch-show-worker)))
+    (notmuch-show-worker)
+
+    ;; Mark the first open message read
+    (notmuch-show-mark-read)))
 
 (defun notmuch-show-worker ()
   (let ((inhibit-read-only t))
@@ -1081,9 +1084,7 @@ function is used."
       (notmuch-show-next-open-message))
 
     ;; Set the header line to the subject of the first open message.
-    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-pretty-subject)))
-
-    (notmuch-show-mark-read)))
+    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-pretty-subject)))))
 
 (defun notmuch-show-capture-state ()
   "Capture the state of the current buffer.
@@ -1132,7 +1133,10 @@ buffer is stored and re-applied after the refresh."
     (erase-buffer)
     (notmuch-show-worker)
     (if state
-	(notmuch-show-apply-state state))))
+	(notmuch-show-apply-state state)
+      ;; Refreshing with state reset navigates to the first open
+      ;; message, so mark read like any other navigation operation.
+      (notmuch-show-mark-read))))
 
 (defvar notmuch-show-stash-map
   (let ((map (make-sparse-keymap)))
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 3/4] emacs: Reverse the meaning of notmuch-show-refresh-view's argument
  2012-02-19 18:02 ` [PATCH v2 0/4] " Austin Clements
  2012-02-19 18:02   ` [PATCH v2 1/4] emacs: Fix out-of-date declare-function Austin Clements
  2012-02-19 18:02   ` [PATCH v2 2/4] emacs: When refreshing a show buffer, only mark read when resetting state Austin Clements
@ 2012-02-19 18:02   ` Austin Clements
  2012-02-19 18:02   ` [PATCH v2 4/4] News for retaining state when refreshing notmuch show Austin Clements
  2012-02-19 19:01   ` [PATCH v2 0/4] Make notmuch-show-refresh-view retain state by default Austin Clements
  4 siblings, 0 replies; 23+ messages in thread
From: Austin Clements @ 2012-02-19 18:02 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

Consensus seems to be that people prefer that refreshing show buffers
retains state by default, rather than resetting it by default.  This
turns out to be the case in the code, as well.  In fact, there's even
a test for this that's been marked broken for several months, which
this patch finally gets to mark as fixed.
---
 emacs/notmuch-crypto.el |    4 ++--
 emacs/notmuch-show.el   |   18 +++++++++---------
 test/emacs              |    1 -
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index 972f26e..94da325 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -120,7 +120,7 @@ mode."
      :notmuch-from from)
     (insert "\n")))
 
-(declare-function notmuch-show-refresh-view "notmuch-show" (&optional retain-state))
+(declare-function notmuch-show-refresh-view "notmuch-show" (&optional reset-state))
 
 (defun notmuch-crypto-sigstatus-good-callback (button)
   (let* ((sigstatus (button-get button :notmuch-sigstatus))
@@ -145,7 +145,7 @@ mode."
 	(insert "\n")
 	(call-process "gpg" nil t t "--list-keys" keyid))
       (recenter -1))
-    (notmuch-show-refresh-view)))
+    (notmuch-show-refresh-view t)))
 
 (defun notmuch-crypto-insert-encstatus-button (encstatus)
   (let* ((status (plist-get encstatus :status))
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index d036c54..ef81cc7 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -957,7 +957,7 @@ current buffer, if possible."
   (message (if notmuch-show-process-crypto
 	       "Processing cryptographic MIME parts."
 	     "Not processing cryptographic MIME parts."))
-  (notmuch-show-refresh-view t))
+  (notmuch-show-refresh-view))
 
 (defun notmuch-show-toggle-elide-non-matching ()
   "Toggle the display of non-matching messages."
@@ -966,7 +966,7 @@ current buffer, if possible."
   (message (if notmuch-show-elide-non-matching-messages
 	       "Showing matching messages only."
 	     "Showing all messages."))
-  (notmuch-show-refresh-view t))
+  (notmuch-show-refresh-view))
 
 (defun notmuch-show-toggle-thread-indentation ()
   "Toggle the indentation of threads."
@@ -975,7 +975,7 @@ current buffer, if possible."
   (message (if notmuch-show-indent-content
 	       "Content is indented."
 	     "Content is not indented."))
-  (notmuch-show-refresh-view t))
+  (notmuch-show-refresh-view))
 
 (defun notmuch-show-insert-tree (tree depth)
   "Insert the message tree TREE at depth DEPTH in the current thread."
@@ -1119,17 +1119,17 @@ This includes:
       (message "Previously current message not found."))
     (notmuch-show-message-adjust)))
 
-(defun notmuch-show-refresh-view (&optional retain-state)
+(defun notmuch-show-refresh-view (&optional reset-state)
   "Refresh the current view.
 
 Refreshes the current view, observing changes in display
-preferences. If RETAIN-STATE is non-nil then the state of the
-buffer is stored and re-applied after the refresh."
+preferences. If invoked with a prefix argument (or RESET-STATE is
+non-nil) then the state of the buffer (open/closed messages) is
+reset based on the original query."
   (interactive "P")
   (let ((inhibit-read-only t)
-	state)
-    (if retain-state
-	(setq state (notmuch-show-capture-state)))
+	(state (unless reset-state
+		 (notmuch-show-capture-state))))
     (erase-buffer)
     (notmuch-show-worker)
     (if state
diff --git a/test/emacs b/test/emacs
index b74cfa9..2dffee8 100755
--- a/test/emacs
+++ b/test/emacs
@@ -456,7 +456,6 @@ test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Refresh modified show buffer"
-test_subtest_known_broken
 test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
 	    (notmuch-show-toggle-message)
 	    (notmuch-show-next-message)
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 4/4] News for retaining state when refreshing notmuch show
  2012-02-19 18:02 ` [PATCH v2 0/4] " Austin Clements
                     ` (2 preceding siblings ...)
  2012-02-19 18:02   ` [PATCH v2 3/4] emacs: Reverse the meaning of notmuch-show-refresh-view's argument Austin Clements
@ 2012-02-19 18:02   ` Austin Clements
  2012-02-19 19:01   ` [PATCH v2 0/4] Make notmuch-show-refresh-view retain state by default Austin Clements
  4 siblings, 0 replies; 23+ messages in thread
From: Austin Clements @ 2012-02-19 18:02 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

---
 NEWS |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/NEWS b/NEWS
index e9abb86..75fa2f4 100644
--- a/NEWS
+++ b/NEWS
@@ -63,6 +63,11 @@ More flexible and consistent tagging operations
 
     (notmuch-show-tag-message "-unread")
 
+Refreshing the show view ('=' by default) no longer opens or closes messages
+
+  To get the old behavior of putting messages back in their initial
+  opened/closed state, use a prefix argument, e.g., C-u =.
+
 Library changes
 ---------------
 
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 0/3] Make notmuch-show-refresh-view retain state by default
  2012-02-19 17:29     ` Dmitry Kurochkin
@ 2012-02-19 18:03       ` Austin Clements
  0 siblings, 0 replies; 23+ messages in thread
From: Austin Clements @ 2012-02-19 18:03 UTC (permalink / raw)
  To: Dmitry Kurochkin; +Cc: Tomi Ollila, notmuch

Quoth Dmitry Kurochkin on Feb 19 at  9:29 pm:
> On Sun, 19 Feb 2012 12:25:41 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > Quoth Tomi Ollila on Feb 19 at 11:43 am:
> > > On Sun, 19 Feb 2012 01:22:10 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > > > Based on the thread at id:"20120213152858.GO27039@mit.edu" it seems
> > > > like people want show refresh to retain message state by default (I
> > > > certainly do), rather than reset it by default.  As a nice bonus, this
> > > > fixes a broken test.
> > > 
> > > Hmm
> > > 
> > > Every '=' keypress in a thread removes 'unread' tag from next unread 
> > > message :o
> > 
> > Oh dear.  It's slightly subtler than that, though.  Refreshing will
> > remove the unread tag from the first message matching the query.  (I
> > suspect you had tag:unread in your query?)  This is particularly
> > confusing for state-retaining refresh because that message might not
> > even be open.
> > 
> > > That is probably desirable feature when refreshing view without
> > > retaining state. Also, probably notmuch-show-refresh-view should
> > > be fixed not to mark next message unread when retaining state ?
> > 
> > I would argue that refresh should never have side-effects, so neither
> > case should mark anything read.  I'll send a v2.
> 
> I agree.  But this looks like a separate issue, no?  So why v2 instead
> of a separate patch/series?

I wanted to slip it in before inverting the default behavior of
refresh.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 0/4] Make notmuch-show-refresh-view retain state by default
  2012-02-19 18:02 ` [PATCH v2 0/4] " Austin Clements
                     ` (3 preceding siblings ...)
  2012-02-19 18:02   ` [PATCH v2 4/4] News for retaining state when refreshing notmuch show Austin Clements
@ 2012-02-19 19:01   ` Austin Clements
  4 siblings, 0 replies; 23+ messages in thread
From: Austin Clements @ 2012-02-19 19:01 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

Quoth myself on Feb 19 at  1:02 pm:
> v2 adds patch 2, which should address Tomi's bug where refreshing
> would mark unexpected messages read.  The other three patches have not
> changed.

Sorry for the spam.  Tomi's bug was subtler than I thought, so v2
didn't actually fix it.  Please ignore v2.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 1/4] emacs: Fix out-of-date declare-function
  2012-02-19 18:02   ` [PATCH v2 1/4] emacs: Fix out-of-date declare-function Austin Clements
@ 2012-02-21  3:09     ` David Bremner
  0 siblings, 0 replies; 23+ messages in thread
From: David Bremner @ 2012-02-21  3:09 UTC (permalink / raw)
  To: Austin Clements, notmuch; +Cc: tomi.ollila

On Sun, 19 Feb 2012 13:02:40 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> The names of the arguments to notmuch-show-refresh-view had gotten out
> of sync between the declare-function and the real thing.

pushed

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v3 0/3] Fix refreshing with state and make it the default
  2012-02-19  6:22 [PATCH 0/3] Make notmuch-show-refresh-view retain state by default Austin Clements
                   ` (5 preceding siblings ...)
  2012-02-19 18:02 ` [PATCH v2 0/4] " Austin Clements
@ 2012-02-21 15:42 ` Austin Clements
  2012-02-21 15:42   ` [PATCH v3 1/3] emacs: When refreshing a show buffer, only mark read when resetting state Austin Clements
                     ` (5 more replies)
  6 siblings, 6 replies; 23+ messages in thread
From: Austin Clements @ 2012-02-21 15:42 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

Third time's the charm.  This fixes show refresh to not mark anything
read when keeping state while refreshing and makes it the default.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v3 1/3] emacs: When refreshing a show buffer, only mark read when resetting state
  2012-02-21 15:42 ` [PATCH v3 0/3] Fix refreshing with state and make it the default Austin Clements
@ 2012-02-21 15:42   ` Austin Clements
  2012-02-21 15:42   ` [PATCH v3 2/3] emacs: Reverse the meaning of notmuch-show-refresh-view's argument Austin Clements
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 23+ messages in thread
From: Austin Clements @ 2012-02-21 15:42 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

If we retain state while refreshing a show buffer, it should not mark
any messages read since it's not a navigation operation (it especially
shouldn't mark the first message matching the query read, which is
what it did previously).  If the user or caller requests that refresh
reset the state of the buffer, then we consider that a navigation
operation, so we do mark the message under point after the refresh
read.

This is implemented by moving responsibility for initial positioning
and read-marking out of notmuch-show-worker and into its caller.
Since notmuch-show-worker is now exclusively about building the show
buffer, we rename it to notmuch-show-build-buffer.
---
 emacs/notmuch-show.el |   26 +++++++++++++++-----------
 1 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index aa9ccee..f759351 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1047,9 +1047,14 @@ function is used."
     (setq notmuch-show-thread-id thread-id
 	  notmuch-show-parent-buffer parent-buffer
 	  notmuch-show-query-context query-context)
-    (notmuch-show-worker)))
+    (notmuch-show-build-buffer)
 
-(defun notmuch-show-worker ()
+    ;; Move to the first open message and mark it read
+    (if (notmuch-show-message-visible-p)
+	(notmuch-show-mark-read)
+      (notmuch-show-next-open-message))))
+
+(defun notmuch-show-build-buffer ()
   (let ((inhibit-read-only t))
 
     (notmuch-show-mode)
@@ -1076,14 +1081,8 @@ function is used."
 
       (run-hooks 'notmuch-show-hook))
 
-    ;; Move straight to the first open message
-    (unless (notmuch-show-message-visible-p)
-      (notmuch-show-next-open-message))
-
     ;; Set the header line to the subject of the first open message.
-    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-pretty-subject)))
-
-    (notmuch-show-mark-read)))
+    (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-pretty-subject)))))
 
 (defun notmuch-show-capture-state ()
   "Capture the state of the current buffer.
@@ -1130,9 +1129,14 @@ buffer is stored and re-applied after the refresh."
     (if retain-state
 	(setq state (notmuch-show-capture-state)))
     (erase-buffer)
-    (notmuch-show-worker)
+    (notmuch-show-build-buffer)
     (if state
-	(notmuch-show-apply-state state))))
+	(notmuch-show-apply-state state)
+      ;; We're resetting state, so navigate to the first open message
+      ;; and mark it read, just like opening a new show buffer.
+      (if (notmuch-show-message-visible-p)
+	  (notmuch-show-mark-read)
+	(notmuch-show-next-open-message)))))
 
 (defvar notmuch-show-stash-map
   (let ((map (make-sparse-keymap)))
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v3 2/3] emacs: Reverse the meaning of notmuch-show-refresh-view's argument
  2012-02-21 15:42 ` [PATCH v3 0/3] Fix refreshing with state and make it the default Austin Clements
  2012-02-21 15:42   ` [PATCH v3 1/3] emacs: When refreshing a show buffer, only mark read when resetting state Austin Clements
@ 2012-02-21 15:42   ` Austin Clements
  2012-02-21 15:42   ` [PATCH v3 3/3] News for retaining state when refreshing notmuch show Austin Clements
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 23+ messages in thread
From: Austin Clements @ 2012-02-21 15:42 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

Consensus seems to be that people prefer that refreshing show buffers
retains state by default, rather than resetting it by default.  This
turns out to be the case in the code, as well.  In fact, there's even
a test for this that's been marked broken for several months, which
this patch finally gets to mark as fixed.
---
 emacs/notmuch-crypto.el |    4 ++--
 emacs/notmuch-show.el   |   18 +++++++++---------
 test/emacs              |    1 -
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index 972f26e..94da325 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -120,7 +120,7 @@ mode."
      :notmuch-from from)
     (insert "\n")))
 
-(declare-function notmuch-show-refresh-view "notmuch-show" (&optional retain-state))
+(declare-function notmuch-show-refresh-view "notmuch-show" (&optional reset-state))
 
 (defun notmuch-crypto-sigstatus-good-callback (button)
   (let* ((sigstatus (button-get button :notmuch-sigstatus))
@@ -145,7 +145,7 @@ mode."
 	(insert "\n")
 	(call-process "gpg" nil t t "--list-keys" keyid))
       (recenter -1))
-    (notmuch-show-refresh-view)))
+    (notmuch-show-refresh-view t)))
 
 (defun notmuch-crypto-insert-encstatus-button (encstatus)
   (let* ((status (plist-get encstatus :status))
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index f759351..ac9bdbc 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -957,7 +957,7 @@ current buffer, if possible."
   (message (if notmuch-show-process-crypto
 	       "Processing cryptographic MIME parts."
 	     "Not processing cryptographic MIME parts."))
-  (notmuch-show-refresh-view t))
+  (notmuch-show-refresh-view))
 
 (defun notmuch-show-toggle-elide-non-matching ()
   "Toggle the display of non-matching messages."
@@ -966,7 +966,7 @@ current buffer, if possible."
   (message (if notmuch-show-elide-non-matching-messages
 	       "Showing matching messages only."
 	     "Showing all messages."))
-  (notmuch-show-refresh-view t))
+  (notmuch-show-refresh-view))
 
 (defun notmuch-show-toggle-thread-indentation ()
   "Toggle the indentation of threads."
@@ -975,7 +975,7 @@ current buffer, if possible."
   (message (if notmuch-show-indent-content
 	       "Content is indented."
 	     "Content is not indented."))
-  (notmuch-show-refresh-view t))
+  (notmuch-show-refresh-view))
 
 (defun notmuch-show-insert-tree (tree depth)
   "Insert the message tree TREE at depth DEPTH in the current thread."
@@ -1117,17 +1117,17 @@ This includes:
       (message "Previously current message not found."))
     (notmuch-show-message-adjust)))
 
-(defun notmuch-show-refresh-view (&optional retain-state)
+(defun notmuch-show-refresh-view (&optional reset-state)
   "Refresh the current view.
 
 Refreshes the current view, observing changes in display
-preferences. If RETAIN-STATE is non-nil then the state of the
-buffer is stored and re-applied after the refresh."
+preferences. If invoked with a prefix argument (or RESET-STATE is
+non-nil) then the state of the buffer (open/closed messages) is
+reset based on the original query."
   (interactive "P")
   (let ((inhibit-read-only t)
-	state)
-    (if retain-state
-	(setq state (notmuch-show-capture-state)))
+	(state (unless reset-state
+		 (notmuch-show-capture-state))))
     (erase-buffer)
     (notmuch-show-build-buffer)
     (if state
diff --git a/test/emacs b/test/emacs
index b74cfa9..2dffee8 100755
--- a/test/emacs
+++ b/test/emacs
@@ -456,7 +456,6 @@ test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Refresh modified show buffer"
-test_subtest_known_broken
 test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
 	    (notmuch-show-toggle-message)
 	    (notmuch-show-next-message)
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v3 3/3] News for retaining state when refreshing notmuch show
  2012-02-21 15:42 ` [PATCH v3 0/3] Fix refreshing with state and make it the default Austin Clements
  2012-02-21 15:42   ` [PATCH v3 1/3] emacs: When refreshing a show buffer, only mark read when resetting state Austin Clements
  2012-02-21 15:42   ` [PATCH v3 2/3] emacs: Reverse the meaning of notmuch-show-refresh-view's argument Austin Clements
@ 2012-02-21 15:42   ` Austin Clements
  2012-02-21 16:34   ` [PATCH v3 0/3] Fix refreshing with state and make it the default Tomi Ollila
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 23+ messages in thread
From: Austin Clements @ 2012-02-21 15:42 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

---
 NEWS |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/NEWS b/NEWS
index e9abb86..75fa2f4 100644
--- a/NEWS
+++ b/NEWS
@@ -63,6 +63,11 @@ More flexible and consistent tagging operations
 
     (notmuch-show-tag-message "-unread")
 
+Refreshing the show view ('=' by default) no longer opens or closes messages
+
+  To get the old behavior of putting messages back in their initial
+  opened/closed state, use a prefix argument, e.g., C-u =.
+
 Library changes
 ---------------
 
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/3] Fix refreshing with state and make it the default
  2012-02-21 15:42 ` [PATCH v3 0/3] Fix refreshing with state and make it the default Austin Clements
                     ` (2 preceding siblings ...)
  2012-02-21 15:42   ` [PATCH v3 3/3] News for retaining state when refreshing notmuch show Austin Clements
@ 2012-02-21 16:34   ` Tomi Ollila
  2012-02-22 19:37   ` Pieter Praet
  2012-02-25 14:39   ` David Bremner
  5 siblings, 0 replies; 23+ messages in thread
From: Tomi Ollila @ 2012-02-21 16:34 UTC (permalink / raw)
  To: Austin Clements, notmuch

On Tue, 21 Feb 2012 10:42:30 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Third time's the charm.  This fixes show refresh to not mark anything
> read when keeping state while refreshing and makes it the default.

+1. LGTM. Works well.

Tomi

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/3] Fix refreshing with state and make it the default
  2012-02-21 15:42 ` [PATCH v3 0/3] Fix refreshing with state and make it the default Austin Clements
                     ` (3 preceding siblings ...)
  2012-02-21 16:34   ` [PATCH v3 0/3] Fix refreshing with state and make it the default Tomi Ollila
@ 2012-02-22 19:37   ` Pieter Praet
  2012-02-25 14:39   ` David Bremner
  5 siblings, 0 replies; 23+ messages in thread
From: Pieter Praet @ 2012-02-22 19:37 UTC (permalink / raw)
  To: Austin Clements, notmuch; +Cc: tomi.ollila

On Tue, 21 Feb 2012 10:42:30 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Third time's the charm.  This fixes show refresh to not mark anything
> read when keeping state while refreshing and makes it the default.
> 

LGTM.


Peace

-- 
Pieter

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/3] Fix refreshing with state and make it the default
  2012-02-21 15:42 ` [PATCH v3 0/3] Fix refreshing with state and make it the default Austin Clements
                     ` (4 preceding siblings ...)
  2012-02-22 19:37   ` Pieter Praet
@ 2012-02-25 14:39   ` David Bremner
  5 siblings, 0 replies; 23+ messages in thread
From: David Bremner @ 2012-02-25 14:39 UTC (permalink / raw)
  To: Austin Clements, notmuch; +Cc: tomi.ollila

On Tue, 21 Feb 2012 10:42:30 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Third time's the charm.  This fixes show refresh to not mark anything
> read when keeping state while refreshing and makes it the default.
> 

Pushed.

d

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2012-02-26  6:02 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-19  6:22 [PATCH 0/3] Make notmuch-show-refresh-view retain state by default Austin Clements
2012-02-19  6:22 ` [PATCH 1/3] emacs: Fix out-of-date declare-function Austin Clements
2012-02-19  6:22 ` [PATCH 2/3] emacs: Reverse the meaning of notmuch-show-refresh-view's argument Austin Clements
2012-02-19  6:22 ` [PATCH 3/3] News for retaining state when refreshing notmuch show Austin Clements
2012-02-19  9:43 ` [PATCH 0/3] Make notmuch-show-refresh-view retain state by default Tomi Ollila
2012-02-19 17:25   ` Austin Clements
2012-02-19 17:29     ` Dmitry Kurochkin
2012-02-19 18:03       ` Austin Clements
2012-02-19 17:16 ` Dmitry Kurochkin
2012-02-19 18:02 ` [PATCH v2 0/4] " Austin Clements
2012-02-19 18:02   ` [PATCH v2 1/4] emacs: Fix out-of-date declare-function Austin Clements
2012-02-21  3:09     ` David Bremner
2012-02-19 18:02   ` [PATCH v2 2/4] emacs: When refreshing a show buffer, only mark read when resetting state Austin Clements
2012-02-19 18:02   ` [PATCH v2 3/4] emacs: Reverse the meaning of notmuch-show-refresh-view's argument Austin Clements
2012-02-19 18:02   ` [PATCH v2 4/4] News for retaining state when refreshing notmuch show Austin Clements
2012-02-19 19:01   ` [PATCH v2 0/4] Make notmuch-show-refresh-view retain state by default Austin Clements
2012-02-21 15:42 ` [PATCH v3 0/3] Fix refreshing with state and make it the default Austin Clements
2012-02-21 15:42   ` [PATCH v3 1/3] emacs: When refreshing a show buffer, only mark read when resetting state Austin Clements
2012-02-21 15:42   ` [PATCH v3 2/3] emacs: Reverse the meaning of notmuch-show-refresh-view's argument Austin Clements
2012-02-21 15:42   ` [PATCH v3 3/3] News for retaining state when refreshing notmuch show Austin Clements
2012-02-21 16:34   ` [PATCH v3 0/3] Fix refreshing with state and make it the default Tomi Ollila
2012-02-22 19:37   ` Pieter Praet
2012-02-25 14:39   ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).