[PATCH v5 0/3] Add filter to emacs show mode

[PATCH v5 0/3] Add filter to emacs show mode

[Mark Walters]

The previous version of this patch was over 3 years ago at
id:1335658006-20161-1-git-send-email-markwalters1009@gmail.com

The reason the patch stopped was that it marked messages read (removed
the unread tag) spuriously (see Austin's review
id:20120429005736.GK2704@mit.edu) and this was difficult to
fix. However, now the unread handling has been updated this is no
longer a problem. (More precisely, since the unread handling is done
in the command loop hook the unread tag only gets updated after
notmuch has displayed the final result.)

There are two small questions with this version that I would
appreciate views on. First it binds the filter/limit to l since f is
already bound to forward in show mode. It changes the binding in
search mode to match but that may annoy a lot of people who have got f
for filter into muscle memory. If we prefer to leave the keybinding in
search mode as it is then we can just drop the final patch.

Secondly what should the behaviour be if the user just gives and empty
filter query? This patch just opens all messages in the thread (ie it
treats the query as matching everything). Alternatively, it could just
exit without refreshing the buffer in that case.

Best wishes

Mark


Mark Walters (3):
  emacs: split notmuch-show-apply-state
  emacs: add a filter option to show
  emacs: Bind filter in search to 'l'

 emacs/notmuch-show.el | 30 +++++++++++++++++++++++-------
 emacs/notmuch.el      |  4 ++--
 2 files changed, 25 insertions(+), 9 deletions(-)

-- 
2.1.4

[PATCH v5 1/3] emacs: split notmuch-show-apply-state

[Mark Walters]

Separate out a notmuch-show-goto-msg-id sub-function from
notmuch-show-apply-state. There should be no functional change but the
next patch will call the new function.
---
 emacs/notmuch-show.el | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 2a53461..a978d3c 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1281,6 +1281,16 @@ (defun notmuch-show-get-query ()
 	      ")")
     notmuch-show-thread-id))
 
+(defun notmuch-show-goto-message (msg-id)
+  "Go to message with msg-id."
+  (goto-char (point-min))
+  (unless (loop if (string= msg-id (notmuch-show-get-message-id))
+		return t
+		until (not (notmuch-show-goto-message-next)))
+    (goto-char (point-min))
+    (message "Message-id not found."))
+  (notmuch-show-message-adjust))
+
 (defun notmuch-show-apply-state (state)
   "Apply STATE to the current buffer.
 
@@ -1298,13 +1308,7 @@ (defun notmuch-show-apply-state (state)
 	  until (not (notmuch-show-goto-message-next)))
 
     ;; Go to the previously open message.
-    (goto-char (point-min))
-    (unless (loop if (string= current (notmuch-show-get-message-id))
-		  return t
-		  until (not (notmuch-show-goto-message-next)))
-      (goto-char (point-min))
-      (message "Previously current message not found."))
-    (notmuch-show-message-adjust)))
+    (notmuch-show-goto-message current)))
 
 (defun notmuch-show-refresh-view (&optional reset-state)
   "Refresh the current view.
-- 
2.1.4

[PATCH v5 2/3] emacs: add a filter option to show

[Mark Walters]

Show the current thread with a different filter (i.e., open messages
in the thread matching the new query).

Bound to 'l' for "limit".

Note that it is not the same as filter in search mode as it replaces
the existing query rather than ANDing with it (but it does keep the
thread-id part of the query).
---
 emacs/notmuch-show.el | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index a978d3c..4dee34b 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -47,6 +47,7 @@
 (declare-function notmuch-tree "notmuch-tree"
 		  (&optional query query-context target buffer-name open-target))
 (declare-function notmuch-tree-get-message-properties "notmuch-tree" nil)
+(declare-function notmuch-read-query "notmuch" (prompt))
 
 (defcustom notmuch-message-headers '("Subject" "To" "Cc" "Date")
   "Headers that should be shown in a message, in this order.
@@ -1372,6 +1373,7 @@ (defvar notmuch-show-mode-map
     (define-key map (kbd "<backtab>") 'notmuch-show-previous-button)
     (define-key map (kbd "TAB") 'notmuch-show-next-button)
     (define-key map "f" 'notmuch-show-forward-message)
+    (define-key map "l" 'notmuch-show-filter-thread)
     (define-key map "r" 'notmuch-show-reply-sender)
     (define-key map "R" 'notmuch-show-reply)
     (define-key map "|" 'notmuch-show-pipe-message)
@@ -1660,6 +1662,16 @@ (defun notmuch-show-command-hook ()
     (save-excursion
       (funcall notmuch-show-mark-read-function (window-start) (window-end)))))
 
+(defun notmuch-show-filter-thread (query)
+  "Filter or LIMIT the current thread based on a new query string.
+
+Reshows the current thread with matches defined by the new query-string."
+  (interactive (list (notmuch-read-query "Filter thread: ")))
+  (let ((msg-id (notmuch-show-get-message-id)))
+    (setq notmuch-show-query-context (if (string= query "") nil query))
+    (notmuch-show-refresh-view t)
+    (notmuch-show-goto-message msg-id)))
+
 ;; Functions for getting attributes of several messages in the current
 ;; thread.
 
-- 
2.1.4

[PATCH v5 3/3] emacs: Bind filter in search to 'l'

[Mark Walters]

Change the key binding for filter (or "limit") in search-mode. This
gives consistency with the new filter in show-mode, and frees 'f' for
forward-thread in the future.
---
 emacs/notmuch.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 6564816..06e3b94 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -166,7 +166,7 @@ (defvar notmuch-search-mode-map
     (define-key map "o" 'notmuch-search-toggle-order)
     (define-key map "c" 'notmuch-search-stash-map)
     (define-key map "t" 'notmuch-search-filter-by-tag)
-    (define-key map "f" 'notmuch-search-filter)
+    (define-key map "l" 'notmuch-search-filter)
     (define-key map [mouse-1] 'notmuch-search-show-thread)
     (define-key map "*" 'notmuch-search-tag-all)
     (define-key map "a" 'notmuch-search-archive-thread)
@@ -983,7 +983,7 @@ (defun notmuch-search-toggle-order ()
   (notmuch-search-refresh-view))
 
 (defun notmuch-search-filter (query)
-  "Filter the current search results based on an additional query string.
+  "Filter or LIMIT the current search results based on an additional query string.
 
 Runs a new search matching only messages that match both the
 current search results AND the additional query string provided."
-- 
2.1.4

Re: [PATCH v5 2/3] emacs: add a filter option to show

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> Show the current thread with a different filter (i.e., open messages
> in the thread matching the new query).
>
> Bound to 'l' for "limit".
>
> Note that it is not the same as filter in search mode as it replaces
> the existing query rather than ANDing with it (but it does keep the
> thread-id part of the query).

I pushed the first two. I'm not sure there was ever any discussion about
the keybindings change, so I left the third.

Re: [PATCH v5 3/3] emacs: Bind filter in search to 'l'

[Tomi Ollila]

On Sat, Jun 13 2015, Mark Walters <markwalters1009@gmail.com> wrote:

> Change the key binding for filter (or "limit") in search-mode. This
> gives consistency with the new filter in show-mode, and frees 'f' for
> forward-thread in the future.
> ---
>  emacs/notmuch.el | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 6564816..06e3b94 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -166,7 +166,7 @@ (defvar notmuch-search-mode-map
>      (define-key map "o" 'notmuch-search-toggle-order)
>      (define-key map "c" 'notmuch-search-stash-map)
>      (define-key map "t" 'notmuch-search-filter-by-tag)
> -    (define-key map "f" 'notmuch-search-filter)
> +    (define-key map "l" 'notmuch-search-filter)

After trying 'f' in id:m2r3maiwmu.fsf@guru.guru-group.fi I'd say I would
have thought 'l' more intuitive choice.

Tomi


>      (define-key map [mouse-1] 'notmuch-search-show-thread)
>      (define-key map "*" 'notmuch-search-tag-all)
>      (define-key map "a" 'notmuch-search-archive-thread)
> @@ -983,7 +983,7 @@ (defun notmuch-search-toggle-order ()
>    (notmuch-search-refresh-view))
>  
>  (defun notmuch-search-filter (query)
> -  "Filter the current search results based on an additional query string.
> +  "Filter or LIMIT the current search results based on an additional query string.
>  
>  Runs a new search matching only messages that match both the
>  current search results AND the additional query string provided."
> -- 
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v5 3/3] emacs: Bind filter in search to 'l'

[David Edmondson]

On Sat, Jun 13 2015, Mark Walters wrote:
> Change the key binding for filter (or "limit") in search-mode. This
> gives consistency with the new filter in show-mode, and frees 'f' for
> forward-thread in the future.

Is there a conclusion on this change? The patch itself seems obviously
fine from a code perspective, at issue is more whether it's acceptable
to change the key binding.

The equivalent binding in notmuch-show mode is already "l", and I think
that it makes sense to be consistent (i.e. the change should be
applied).

> ---
>  emacs/notmuch.el | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 6564816..06e3b94 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -166,7 +166,7 @@ (defvar notmuch-search-mode-map
>      (define-key map "o" 'notmuch-search-toggle-order)
>      (define-key map "c" 'notmuch-search-stash-map)
>      (define-key map "t" 'notmuch-search-filter-by-tag)
> -    (define-key map "f" 'notmuch-search-filter)
> +    (define-key map "l" 'notmuch-search-filter)
>      (define-key map [mouse-1] 'notmuch-search-show-thread)
>      (define-key map "*" 'notmuch-search-tag-all)
>      (define-key map "a" 'notmuch-search-archive-thread)
> @@ -983,7 +983,7 @@ (defun notmuch-search-toggle-order ()
>    (notmuch-search-refresh-view))
>  
>  (defun notmuch-search-filter (query)
> -  "Filter the current search results based on an additional query string.
> +  "Filter or LIMIT the current search results based on an additional query string.
>  
>  Runs a new search matching only messages that match both the
>  current search results AND the additional query string provided."
> -- 
> 2.1.4

Re: [PATCH v5 3/3] emacs: Bind filter in search to 'l'

[Mark Walters]

On Tue, 09 Feb 2016, David Edmondson <dme@dme.org> wrote:
> On Sat, Jun 13 2015, Mark Walters wrote:
>> Change the key binding for filter (or "limit") in search-mode. This
>> gives consistency with the new filter in show-mode, and frees 'f' for
>> forward-thread in the future.
>
> Is there a conclusion on this change? The patch itself seems obviously
> fine from a code perspective, at issue is more whether it's acceptable
> to change the key binding.
>
> The equivalent binding in notmuch-show mode is already "l", and I think
> that it makes sense to be consistent (i.e. the change should be
> applied).

I broadly agree, particularly as it will free up f for forwarding a
thread in search view (which is easy to do on top of
id:1454931262-21362-1-git-send-email-dme@dme.org).

It will be annoying in the short term to learn the change, but I don't
see a way round that.

Best wishes

Mark




>
>> ---
>>  emacs/notmuch.el | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>> index 6564816..06e3b94 100644
>> --- a/emacs/notmuch.el
>> +++ b/emacs/notmuch.el
>> @@ -166,7 +166,7 @@ (defvar notmuch-search-mode-map
>>      (define-key map "o" 'notmuch-search-toggle-order)
>>      (define-key map "c" 'notmuch-search-stash-map)
>>      (define-key map "t" 'notmuch-search-filter-by-tag)
>> -    (define-key map "f" 'notmuch-search-filter)
>> +    (define-key map "l" 'notmuch-search-filter)
>>      (define-key map [mouse-1] 'notmuch-search-show-thread)
>>      (define-key map "*" 'notmuch-search-tag-all)
>>      (define-key map "a" 'notmuch-search-archive-thread)
>> @@ -983,7 +983,7 @@ (defun notmuch-search-toggle-order ()
>>    (notmuch-search-refresh-view))
>>  
>>  (defun notmuch-search-filter (query)
>> -  "Filter the current search results based on an additional query string.
>> +  "Filter or LIMIT the current search results based on an additional query string.
>>  
>>  Runs a new search matching only messages that match both the
>>  current search results AND the additional query string provided."
>> -- 
>> 2.1.4

Re: [PATCH v5 3/3] emacs: Bind filter in search to 'l'

[Tomi Ollila]

On Tue, Feb 09 2016, Mark Walters <markwalters1009@gmail.com> wrote:

> On Tue, 09 Feb 2016, David Edmondson <dme@dme.org> wrote:
>> On Sat, Jun 13 2015, Mark Walters wrote:
>>> Change the key binding for filter (or "limit") in search-mode. This
>>> gives consistency with the new filter in show-mode, and frees 'f' for
>>> forward-thread in the future.
>>
>> Is there a conclusion on this change? The patch itself seems obviously
>> fine from a code perspective, at issue is more whether it's acceptable
>> to change the key binding.
>>
>> The equivalent binding in notmuch-show mode is already "l", and I think
>> that it makes sense to be consistent (i.e. the change should be
>> applied).
>
> I broadly agree, particularly as it will free up f for forwarding a
> thread in search view (which is easy to do on top of
> id:1454931262-21362-1-git-send-email-dme@dme.org).
>
> It will be annoying in the short term to learn the change, but I don't
> see a way round that.

In short, +1 for the change.

Tomi

>
> Best wishes
>
> Mark
>
>>> ---
>>>  emacs/notmuch.el | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>>> index 6564816..06e3b94 100644
>>> --- a/emacs/notmuch.el
>>> +++ b/emacs/notmuch.el
>>> @@ -166,7 +166,7 @@ (defvar notmuch-search-mode-map
>>>      (define-key map "o" 'notmuch-search-toggle-order)
>>>      (define-key map "c" 'notmuch-search-stash-map)
>>>      (define-key map "t" 'notmuch-search-filter-by-tag)
>>> -    (define-key map "f" 'notmuch-search-filter)
>>> +    (define-key map "l" 'notmuch-search-filter)
>>>      (define-key map [mouse-1] 'notmuch-search-show-thread)
>>>      (define-key map "*" 'notmuch-search-tag-all)
>>>      (define-key map "a" 'notmuch-search-archive-thread)
>>> @@ -983,7 +983,7 @@ (defun notmuch-search-toggle-order ()
>>>    (notmuch-search-refresh-view))
>>>  
>>>  (defun notmuch-search-filter (query)
>>> -  "Filter the current search results based on an additional query string.
>>> +  "Filter or LIMIT the current search results based on an additional query string.
>>>  
>>>  Runs a new search matching only messages that match both the
>>>  current search results AND the additional query string provided."
>>> -- 
>>> 2.1.4

Re: [PATCH v5 3/3] emacs: Bind filter in search to 'l'

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> Change the key binding for filter (or "limit") in search-mode. This
> gives consistency with the new filter in show-mode, and frees 'f' for
> forward-thread in the future.

Pushed. It occurs to me this deserves a NEWS entry.

d

Re: [PATCH v5 3/3] emacs: Bind filter in search to 'l'

[Daniel Kahn Gillmor]

On Tue 2016-02-09 16:04:20 -0500, Mark Walters wrote:
> I broadly agree, particularly as it will free up f for forwarding a
> thread in search view (which is easy to do on top of
> id:1454931262-21362-1-git-send-email-dme@dme.org).

Being able to forward a whole thread from emacs would be great.  If we
have that available, it would also be nice to bind it to 'F' in the
notmuch-show view.  (assuming that people don't have objections about
the different cases doing different things)

> It will be annoying in the short term to learn the change, but I don't
> see a way round that.

I think this is a price worth paying.

  --dkg

Re: [PATCH v5 3/3] emacs: Bind filter in search to 'l'

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> Being able to forward a whole thread from emacs would be great.  If we
> have that available, it would also be nice to bind it to 'F' in the
> notmuch-show view.  (assuming that people don't have objections about
> the different cases doing different things)

We already do different things with r and R.

d

Re: [PATCH v5 3/3] emacs: Bind filter in search to 'l'

[David Edmondson]

On Wed, Feb 17 2016, Daniel Kahn Gillmor wrote:
> On Tue 2016-02-09 16:04:20 -0500, Mark Walters wrote:
>> I broadly agree, particularly as it will free up f for forwarding a
>> thread in search view (which is easy to do on top of
>> id:1454931262-21362-1-git-send-email-dme@dme.org).
>
> Being able to forward a whole thread from emacs would be great.  If we
> have that available, it would also be nice to bind it to 'F' in the
> notmuch-show view.  (assuming that people don't have objections about
> the different cases doing different things)

id:1454931262-21362-1-git-send-email-dme@dme.org proposes to bind 'F' to
"forward all open messages" in notmuch-show view.