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

[Matt Armstrong]

Mark Walters <markwalters1009 at gmail.com> writes:

> Hi
>
> On Fri, 16 Sep 2016, Matt Armstrong <marmstrong at 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 at 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 at 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.