Hi On Wed, Nov 7, 2012 at 12:14 AM, Mark Walters <markwalters1009@gmail.com> wrote: > This is not a full review: just a couple of thoughts. It basically seems > to work as expected. I am not quite sure what behaviour you would expect > in a couple of corner cases: > > 1) what it the user toggles the display of matching messages (elide > mode)? Do you still want all tags from all messages including those not > visible? > 2) What about any tags from excluded messages? Should they show up? What > if they would be excluded but aren't because of the particular search? to me, a thread's tags should be independent of what is visible and thus should be stable. For example, when I want to star a thread, I star the message I'm currently seeing and I expect the thread to be starred for the rest of its life, even if the particular mail is not currently visible. What is the opinion of others? >> emacs/notmuch-tager.el | 76 +++++++++++++++++++++++++++ > > I would go for tagger rather than tager (but others can disagree). I don't care and can change it without problem. I picked 'tager' because it's shorter and I like to have a lot of information in my function names. What is the opinion of others? >> +(defun notmuch-query-thread-tags-from-id (thread-id) >> + "Return the tags of thread whose id is THREAD-ID. >> +The thread tags are the union of the tags of emails in the >> +thread." >> + (let ((tag-lists >> + (notmuch-query-map-forest >> + (lambda (msg) (plist-get msg :tags)) >> + (car (notmuch-query-get-threads >> + (list (concat "thread:" thread-id))))))) >> + (case (length tag-lists) >> + (0 nil) >> + (1 (car tag-lists)) >> + (otherwise (reduce (lambda (l1 l2) >> + (union l1 l2 :test 'string=)) >> + tag-lists))))) > > Couldn't you do this with notmuch-show-mapc and avoid the extra call to > notmuch? It also probably helps some cases of excluded tags and elide > mode. > That would not work for my definition of a thread's tags. But if we change the definition, I can change the implementation of this function and avoid a call to notmuch. > If for some reason the query is better than I think you work > to remove the thread: from the thread-id below and then add it back in > here? I know and it is on purpose :-). When I first had a look at notmuch sources, I was confused by all those thread-id and mail-id everywhere that are sometimes ids (e.g., "000012") and sometimes queries (e.g., "thread:000012"). To me, the code should use ids everywhere and build a query out of that when calling notmuch or when displaying a query. This would also avoid the use of the optional `bare' parameter in some existing functions. Thank you for your review. So, I'm waiting for more opinions because changing anything. Best -- Damien Cassou http://damiencassou.seasidehosting.st "Success is the ability to go from one failure to another without losing enthusiasm." Winston Churchill