From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Summary by thread in rmail Date: Mon, 10 Oct 2022 10:15:43 +0300 Message-ID: <83bkqki1y8.fsf@gnu.org> References: <87o7uq6ihu.fsf@autistici.org> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="6850"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Andrea Monaco , Richard Stallman Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Mon Oct 10 09:18:07 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1ohn38-0001cN-47 for ged-emacs-devel@m.gmane-mx.org; Mon, 10 Oct 2022 09:18:06 +0200 Original-Received: from localhost ([::1]:42254 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ohn37-000344-5l for ged-emacs-devel@m.gmane-mx.org; Mon, 10 Oct 2022 03:18:05 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:55366) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ohn0n-0001zY-D8 for emacs-devel@gnu.org; Mon, 10 Oct 2022 03:15:41 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:39576) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ohn0m-0007Ek-1P; Mon, 10 Oct 2022 03:15:40 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=CkjGqCWUjZeERECTuhIYwHoqX+XsQyZjuQ/qs5jFJoM=; b=bQPHkL3SWk6s 1fv2jNixTa1OaKf176/KsrOfzzSA6UDHwbnCXaJC/alJknQKEtxJ/AC6Im1uLQS1oTcNS5zr1wdKa JcnDYWZk+90FvL25Kgkamqo183z7KJydm4KGexSkOGXe8ZIyb61sI66IBQnJgRmibVBWQoXuW11+E 9a/sCYe6EueO8yAdICMdoo+Kb3AanA4VYy6Z77T70xwVaDVkatUAVSG4huxddJ9lBHhqUKuz7wyX5 UfkokombIaKk+TGgKj57rxL5Agg+snn8LgTCJWDDHeA0sEz9OPrNKPoNzY3aeNOnNFUnTnrPJ9wft ahL8F7mozt161tsyXNKMXA==; Original-Received: from [87.69.77.57] (port=1979 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ohn0k-0001uV-Ma; Mon, 10 Oct 2022 03:15:39 -0400 In-Reply-To: <87o7uq6ihu.fsf@autistici.org> (message from Andrea Monaco on Wed, 05 Oct 2022 23:57:49 +0200) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:297324 Archived-At: > From: Andrea Monaco > Date: Wed, 05 Oct 2022 23:57:49 +0200 > > I had some code ready. This only looks at the Subject field. You can > invoke it with rmail-thread-summary and it creates a buffer called > e.g. RMAIL-thread-summary that is independent of RMAIL-summary. There's > still no code to update the summary after getting new mail. It's basic, > but it works. Thanks. Please see some comments below. Richard, I'd appreciate your review as well. > --- /dev/null > +++ b/lisp/mail/rmailthread.el Please add this to rmailsum.el, instead of making a new file. > +(defun rmail-get-create-thread-summary-buffer () > + "Return the Rmail thread summary buffer. > +If necessary, it is created and undo is disabled." > + (if (and rmail-thread-summary-buffer (buffer-name rmail-thread-summary-buffer)) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ We nowadays have the buffer-live-p function to make such tests. > + (let ((buff (generate-new-buffer (concat (buffer-name) "-thread-summary")))) I think "-summary-by-thread" is a better name. "Thread-summary" hints that it is a summary of a single thread, which is not the case here. > +(defun rmail-new-thread-summary (desc redo function &rest args) > + "Create a summary of selected messages by thread." The "selected" part here is misleading, I think. The truth is, the messages can be "selected" by the FUNCTION argument, but what kind of FUNCTION can make sense for this summary? And in any case, if we keep the FUNCTION arg, the doc string should explain how messages are "selected" for the summary. > + (with-current-buffer rmail-buffer > + (setq rmail-thread-summary-buffer (rmail-new-thread-summary-1 desc redo function args) > + ;; r-s-b is buffer-local. What is "r-s-b" here? Please don't use abbreviated symbol names, they make reading the code harder. > + ;; This is how rmail makes the summary buffer reappear. > + ;; We do this here to make the window the proper size. > + ;(rmail-select-summary nil) > + ;(set-buffer sumbuf)) Is this commented-out code necessary? > + ;(rmail-summary-goto-msg mesg t t) > + ;(rmail-summary-construct-io-menu) And this? > + (if thread > + (setcdr thread > + (cons (cons msgnum (rmail-get-summary msgnum)) (cdr thread))) > + (progn > + (let* ((newthread (list subject (cons msgnum (rmail-get-summary msgnum)))) > + (newcell (cons newthread cell))) > + (setq cell newcell) > + (puthash subject newcell rmail-thread-hash-table) > + (setq ordered-threads (cons newthread ordered-threads))))) Is progn really necessary here? > + (setq ordered-threads (cons newthread ordered-threads))))) Maybe using 'push' here will make the code more concise and readable? > + (setq msgnum (1+ msgnum)) > + (if (and (not (zerop msgnum)) > + (zerop (% msgnum 10))) > + (message "Computing thread summary lines... %d" > + msgnum)))))) "Computing by-thread summary lines... %d" is a better message, I think. > + (while thread-message > + (let* ((suml (cdar thread-message)) > + (newsuml (concat (substring suml 0 42) " " (substring suml 42)))) Why are you truncating the subject at the 42nd character? and why is 42 hard-coded? This: > + (setq-local minor-mode-alist (list (list t (concat ": " description)))) together with this: > + (rmail-new-thread-summary "All" '(rmail-thread-summary) nil)) Will AFAIU display "RMAIL Summary: All" in the mode line. However, I think the mode line should in this case show something like this instead: RMAIL Summary: By-Thread Thanks again for working on this.