* [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 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
* 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 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
* [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
* 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 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 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
* [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).