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

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

Date: Fri, 16 Sep 2016 20:26:56 +0100

To: Matt Armstrong, notmuch@notmuchmail.org

Cc:

From: 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


Thread: