[PATCH v1] WIP: emacs: show: expand unread tags

[PATCH v1] WIP: emacs: show: expand unread tags

[Matt Armstrong]

Expand unread messages by default in show mode.

Show mode now expands messages matching tags designated by
notmuch-show-unread-tags (in addition to those matching the search
query).  By default, this list is `("unread").  This way, one can still
tag and search for messages however one likes, but if the thread is
displayed those messages matching the search term OR unread are
expanded.

---

See id:qf54m70o7h5.fsf@marmstrong-linux.kir.corp.google.com for a
related discussion thread.

What happens today when notmuch-show does the query to show a thread, it
takes the user's query Q and constructs:

  thread:THREAD and Q

This changes the above to:

  thread:THREAD and ((Q) or (tag:unread))

I've found this to be a much better day to day experience for myself and
am interesting on iterating on this idea until it is can be
committed.  (settling on interface first, then testing)

As I explain in the thread noted above, I found the default notmuch
experience to be quite confusing, at least when processing "inbox" mail.

The problem is that my work experience is based on a large number of
mailing lists that cross post to each other, often initiated mid-thread.
I aggressively archive these threads as "unread", but sometimes a long
thread is cross-posted to my team mailing list, or to me directly.  If
these show up in my "team" or "me" tags, I want to have a clear
indication that I have yet to read the entire thread.  Expanding
"tag:unread" messages makes sense here.

It might make less sense if drilling down for those specific messages.
In that case I wonder if C-u RET (to toggle
notmuch-show-elide-non-matching-messages) is sufficient?
---
 emacs/notmuch-show.el | 59 +++++++++++++++++++++++++++++++++++++++++----------
 emacs/notmuch.el      |  4 ++++
 2 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5a585f3..f20eec6 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -142,10 +142,21 @@ indentation."
 			notmuch-show-interactively-view-part)))
 
 (defcustom notmuch-show-only-matching-messages nil
-  "Only matching messages are shown by default."
+  "When non-nil only matching messages are shown.
+
+See `notmuch-search-show-thread'."
   :type 'boolean
   :group 'notmuch-show)
 
+(defcustom notmuch-show-unread-tags '("unread")
+  "A list of tags to expand by default in `notmuch-show-mode'.
+
+In addition to the search query, any messages that match one of
+the tags in the list are expanded.  Has no effect if
+`notmuch-show-only-matching-messages', which see."
+  :type '(repeat string)
+  :group 'notmuch-show)
+
 ;; By default, block all external images to prevent privacy leaks and
 ;; potential attacks.
 (defcustom notmuch-show-text/html-blocked-images "."
@@ -1258,6 +1269,24 @@ matched."
 	(message "No messages matched the query!")
 	nil))))
 
+(defun notmuch-show--query-or (a b)
+  "Form a query string \"(A) or (B)\".
+
+The args must be strings. If either is nil, returns the other."
+  (cond
+   ((null a) b)
+   ((null b) a)
+   (t (concat "(" a ") or (" b ")"))))
+
+(defun notmuch-show--query-and (a b)
+  "Form a query string \"(A) and (B)\".
+
+The args must be strings. If either is nil, returns the other."
+  (cond
+   ((null a) b)
+   ((null b) a)
+   (t (concat "(" a ") and (" b ")"))))
+
 (defun notmuch-show--build-buffer (&optional state)
   "Display messages matching the current buffer context.
 
@@ -1265,21 +1294,29 @@ Apply the previously saved STATE if supplied, otherwise show the
 first relevant message.
 
 If no messages match the query return NIL."
-  (let* ((basic-args (list notmuch-show-thread-id))
-	 (args (if notmuch-show-query-context
-		   (append (list "\'") basic-args
-			   (list "and (" notmuch-show-query-context ")\'"))
-		 (append (list "\'") basic-args (list "\'"))))
+  (let* ((unread (if notmuch-show-unread-tags
+		     (mapconcat
+		      '(lambda (tag) (concat "tag:" tag))
+		      notmuch-show-unread-tags
+		      " or ")))
+	 (basic-query (notmuch-show--query-or
+		       notmuch-show-thread-id unread))
+	 (context-query (if notmuch-show-query-context
+			    (notmuch-show--query-and
+			     notmuch-show-thread-id
+			     (notmuch-show--query-or
+			      notmuch-show-query-context
+			      unread))))
 	 (cli-args (cons "--exclude=false"
 			 (when notmuch-show-elide-non-matching-messages
 			   (list "--entire-thread=false"))))
-
-	 (forest (or (notmuch-query-get-threads (append cli-args args))
+	 (forest (or (and context-query
+			  (notmuch-query-get-threads
+			   (append cli-args (list "\'" context-query "\'"))))
 		     ;; If a query context reduced the number of
 		     ;; results to zero, try again without it.
-		     (and notmuch-show-query-context
-			  (notmuch-query-get-threads (append cli-args basic-args)))))
-
+		     (notmuch-query-get-threads
+		      (append cli-args (list "\'" basic-query "\'")))))
 	 ;; Must be reset every time we are going to start inserting
 	 ;; messages into the buffer.
 	 (notmuch-show-previous-subject ""))
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 8e14692..7cc9e9e 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -480,6 +480,10 @@ no messages in the region then return nil."
 (defun notmuch-search-show-thread (&optional elide-toggle)
   "Display the currently selected thread.
 
+Messages in the thread matching the query, as well as any in
+`notmuch-show-unread-tags', are expanded.  The others are
+initially displayed collapsed.
+
 With a prefix argument, invert the default value of
 `notmuch-show-only-matching-messages' when displaying the
 thread."
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH v1] WIP: emacs: show: expand unread tags

[Mark Walters]

Hi

On Fri, 16 Sep 2016, Matt Armstrong <marmstrong@google.com> wrote:
> Expand unread messages by default in show mode.
>
> Show mode now expands messages matching tags designated by
> notmuch-show-unread-tags (in addition to those matching the search
> query).  By default, this list is `("unread").  This way, one can still
> tag and search for messages however one likes, but if the thread is
> displayed those messages matching the search term OR unread are
> expanded.

Broadly I like the idea.

> See id:qf54m70o7h5.fsf@marmstrong-linux.kir.corp.google.com for a
> related discussion thread.
>
> What happens today when notmuch-show does the query to show a thread, it
> takes the user's query Q and constructs:
>
>   thread:THREAD and Q
>
> This changes the above to:
>
>   thread:THREAD and ((Q) or (tag:unread))

This seems a reasonable way of doing this. If you are doing it this way
then presumably the extra query (tag:unread in the above) could be any
query, not just a tag query? This might make sense, as it would be
analogous to count-queries in notmuch-hello. 

I will mention a couple of possible extensions as it might be worth
making sure they are possible even if they aren't implemented yet. One
is that the user might want to not open matching messages: that is to
have the thread opened as e.g., thread:THREAD and (tag:unread)

Another is that we might want the this extra search to be settable on a
per saved search basis. Saved searches are a plist so that should be
easy.

> I've found this to be a much better day to day experience for myself and
> am interesting on iterating on this idea until it is can be
> committed.  (settling on interface first, then testing)
>
> As I explain in the thread noted above, I found the default notmuch
> experience to be quite confusing, at least when processing "inbox" mail.
>
> The problem is that my work experience is based on a large number of
> mailing lists that cross post to each other, often initiated mid-thread.
> I aggressively archive these threads as "unread", but sometimes a long
> thread is cross-posted to my team mailing list, or to me directly.  If
> these show up in my "team" or "me" tags, I want to have a clear
> indication that I have yet to read the entire thread.  Expanding
> "tag:unread" messages makes sense here.
>
> It might make less sense if drilling down for those specific messages.
> In that case I wonder if C-u RET (to toggle
> notmuch-show-elide-non-matching-messages) is sufficient?
> ---
>  emacs/notmuch-show.el | 59 +++++++++++++++++++++++++++++++++++++++++----------
>  emacs/notmuch.el      |  4 ++++
>  2 files changed, 52 insertions(+), 11 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 5a585f3..f20eec6 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -142,10 +142,21 @@ indentation."
>  			notmuch-show-interactively-view-part)))
>  
>  (defcustom notmuch-show-only-matching-messages nil
> -  "Only matching messages are shown by default."
> +  "When non-nil only matching messages are shown.
> +
> +See `notmuch-search-show-thread'."
>    :type 'boolean

This seems to be unrelated so perhaps omit (or split into a separate patch).

>    :group 'notmuch-show)
>  
> +(defcustom notmuch-show-unread-tags '("unread")
> +  "A list of tags to expand by default in `notmuch-show-mode'.

Even if you stick with just allowing tags I think the name here should
not involve unread as it is more general than that. However, since it
doesn't appear to be any more difficult, I would suggest that you allow
more general queries.

> +In addition to the search query, any messages that match one of
> +the tags in the list are expanded.  Has no effect if
> +`notmuch-show-only-matching-messages', which see."
> +  :type '(repeat string)
> +  :group 'notmuch-show)
> +
>  ;; By default, block all external images to prevent privacy leaks and
>  ;; potential attacks.
>  (defcustom notmuch-show-text/html-blocked-images "."
> @@ -1258,6 +1269,24 @@ matched."
>  	(message "No messages matched the query!")
>  	nil))))
>  
> +(defun notmuch-show--query-or (a b)
> +  "Form a query string \"(A) or (B)\".
> +
> +The args must be strings. If either is nil, returns the other."
> +  (cond
> +   ((null a) b)
> +   ((null b) a)
> +   (t (concat "(" a ") or (" b ")"))))
> +
> +(defun notmuch-show--query-and (a b)
> +  "Form a query string \"(A) and (B)\".
> +
> +The args must be strings. If either is nil, returns the other."
> +  (cond
> +   ((null a) b)
> +   ((null b) a)
> +   (t (concat "(" a ") and (" b ")"))))
> +
>  (defun notmuch-show--build-buffer (&optional state)
>    "Display messages matching the current buffer context.
>  
> @@ -1265,21 +1294,29 @@ Apply the previously saved STATE if supplied, otherwise show the
>  first relevant message.
>  
>  If no messages match the query return NIL."
> -  (let* ((basic-args (list notmuch-show-thread-id))
> -	 (args (if notmuch-show-query-context
> -		   (append (list "\'") basic-args
> -			   (list "and (" notmuch-show-query-context ")\'"))
> -		 (append (list "\'") basic-args (list "\'"))))
> +  (let* ((unread (if notmuch-show-unread-tags
> +		     (mapconcat
> +		      '(lambda (tag) (concat "tag:" tag))
> +		      notmuch-show-unread-tags
> +		      " or ")))
> +	 (basic-query (notmuch-show--query-or
> +		       notmuch-show-thread-id unread))

I don't understand why the basic query even mentions unread? It surely
shouldn't be ORed -- I think it should just be notmuch-show-thread-id as
before. (The idea is that if there are no longer any matching messages
in the thread we still show the thread.)

> +	 (context-query (if notmuch-show-query-context
> +			    (notmuch-show--query-and
> +			     notmuch-show-thread-id
> +			     (notmuch-show--query-or
> +			      notmuch-show-query-context
> +			      unread))))
>  	 (cli-args (cons "--exclude=false"
>  			 (when notmuch-show-elide-non-matching-messages
>  			   (list "--entire-thread=false"))))
> -
> -	 (forest (or (notmuch-query-get-threads (append cli-args args))
> +	 (forest (or (and context-query
> +			  (notmuch-query-get-threads
> +			   (append cli-args (list "\'" context-query "\'"))))
>  		     ;; If a query context reduced the number of
>  		     ;; results to zero, try again without it.
> -		     (and notmuch-show-query-context
> -			  (notmuch-query-get-threads (append cli-args basic-args)))))
> -
> +		     (notmuch-query-get-threads
> +		      (append cli-args (list "\'" basic-query "\'")))))

We may need to consider how this interacts with the limit functionality
(notmuch-show-filter-thread): if it is always one specific query/tag-set
like you have currently it's probably OK, but if it can vary (e.g., depending
on the saved-search) then it is unclear what should happen when
limiting.

Best wishes

Mark

Re: [PATCH v1] WIP: emacs: show: expand unread tags

[Matt Armstrong]

Mark Walters <markwalters1009@gmail.com> writes:

> Hi
>
> On Fri, 16 Sep 2016, Matt Armstrong <marmstrong@google.com> wrote:
>> Expand unread messages by default in show mode.
>>
>> Show mode now expands messages matching tags designated by
>> notmuch-show-unread-tags (in addition to those matching the search
>> query).  By default, this list is `("unread").  This way, one can still
>> tag and search for messages however one likes, but if the thread is
>> displayed those messages matching the search term OR unread are
>> expanded.
>
> Broadly I like the idea.
>
>> See id:qf54m70o7h5.fsf@marmstrong-linux.kir.corp.google.com for a
>> related discussion thread.
>>
>> What happens today when notmuch-show does the query to show a thread, it
>> takes the user's query Q and constructs:
>>
>>   thread:THREAD and Q
>>
>> This changes the above to:
>>
>>   thread:THREAD and ((Q) or (tag:unread))
>
> This seems a reasonable way of doing this. If you are doing it this way
> then presumably the extra query (tag:unread in the above) could be any
> query, not just a tag query? This might make sense, as it would be
> analogous to count-queries in notmuch-hello. 

I like the idea of a query.  Carl had said he preferred the tag based
approach, but this was when contrasted with an approach where the entire
search query was replaced with an entirely different "expansion query".
See id:qf54m70o7h5.fsf@marmstrong-linux.kir.corp.google.com.

> I will mention a couple of possible extensions as it might be worth
> making sure they are possible even if they aren't implemented yet. One
> is that the user might want to not open matching messages: that is to
> have the thread opened as e.g., thread:THREAD and (tag:unread)
>
> Another is that we might want the this extra search to be settable on a
> per saved search basis. Saved searches are a plist so that should be
> easy.

Yes, these second two suggestions are similar to the ideas Jani
presented on the original discussion thread.  I like them.

>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
>> index 5a585f3..f20eec6 100644
>> --- a/emacs/notmuch-show.el
>> +++ b/emacs/notmuch-show.el
>> @@ -142,10 +142,21 @@ indentation."
>>  			notmuch-show-interactively-view-part)))
>>  
>>  (defcustom notmuch-show-only-matching-messages nil
>> -  "Only matching messages are shown by default."
>> +  "When non-nil only matching messages are shown.
>> +
>> +See `notmuch-search-show-thread'."
>>    :type 'boolean
>
> This seems to be unrelated so perhaps omit (or split into a separate patch).

Agreed, will do.


>>    :group 'notmuch-show)
>>  
>> +(defcustom notmuch-show-unread-tags '("unread")
>> +  "A list of tags to expand by default in `notmuch-show-mode'.
>
> Even if you stick with just allowing tags I think the name here should
> not involve unread as it is more general than that. However, since it
> doesn't appear to be any more difficult, I would suggest that you allow
> more general queries.

Agreed, will do.


>> +In addition to the search query, any messages that match one of
>> +the tags in the list are expanded.  Has no effect if
>> +`notmuch-show-only-matching-messages', which see."
>> +  :type '(repeat string)
>> +  :group 'notmuch-show)
>> +
>>  ;; By default, block all external images to prevent privacy leaks and
>>  ;; potential attacks.
>>  (defcustom notmuch-show-text/html-blocked-images "."
>> @@ -1258,6 +1269,24 @@ matched."
>>  	(message "No messages matched the query!")
>>  	nil))))
>>  
>> +(defun notmuch-show--query-or (a b)
>> +  "Form a query string \"(A) or (B)\".
>> +
>> +The args must be strings. If either is nil, returns the other."
>> +  (cond
>> +   ((null a) b)
>> +   ((null b) a)
>> +   (t (concat "(" a ") or (" b ")"))))
>> +
>> +(defun notmuch-show--query-and (a b)
>> +  "Form a query string \"(A) and (B)\".
>> +
>> +The args must be strings. If either is nil, returns the other."
>> +  (cond
>> +   ((null a) b)
>> +   ((null b) a)
>> +   (t (concat "(" a ") and (" b ")"))))
>> +
>>  (defun notmuch-show--build-buffer (&optional state)
>>    "Display messages matching the current buffer context.
>>  
>> @@ -1265,21 +1294,29 @@ Apply the previously saved STATE if supplied, otherwise show the
>>  first relevant message.
>>  
>>  If no messages match the query return NIL."
>> -  (let* ((basic-args (list notmuch-show-thread-id))
>> -	 (args (if notmuch-show-query-context
>> -		   (append (list "\'") basic-args
>> -			   (list "and (" notmuch-show-query-context ")\'"))
>> -		 (append (list "\'") basic-args (list "\'"))))
>> +  (let* ((unread (if notmuch-show-unread-tags
>> +		     (mapconcat
>> +		      '(lambda (tag) (concat "tag:" tag))
>> +		      notmuch-show-unread-tags
>> +		      " or ")))
>> +	 (basic-query (notmuch-show--query-or
>> +		       notmuch-show-thread-id unread))
>
> I don't understand why the basic query even mentions unread? It surely
> shouldn't be ORed -- I think it should just be notmuch-show-thread-id as
> before. (The idea is that if there are no longer any matching messages
> in the thread we still show the thread.)

Yes, I discovered this today, and fixed it as you suggested: the
fallback basic query is the thread id only.

I don't think the fallback to the basic-query occurs often, but when it
did to me today, it slurped in all the unread mail and hung notmuch and
Emacs for quite some time.

> We may need to consider how this interacts with the limit functionality
> (notmuch-show-filter-thread): if it is always one specific query/tag-set
> like you have currently it's probably OK, but if it can vary (e.g., depending
> on the saved-search) then it is unclear what should happen when
> limiting.

I intend dig into limit to see if the result is sane.

In part I think this gets back to the expected use cases.  I think there
may be two:

a) User is in "find threads to read" mode (i.e. "reading my inbox
mode").  Intent is to be reading incoming mail.  The search query is
primarily identifying interesting threads.  Once identified, the unread
messages are what is most relevant.

b) User is in "find that message" mode.  Here, they're looking for
particular things.  The search query is primary, and unread messages are
not particularly relevant.

I'll think more about this, but also welcome ideas from others.