--- /dev/null
+Return-Path: <jani@nikula.org>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id B785E429E25\r
+ for <notmuch@notmuchmail.org>; Sat, 29 Oct 2011 13:28:21 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.7\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
+ tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id lyk7tatzSOxc for <notmuch@notmuchmail.org>;\r
+ Sat, 29 Oct 2011 13:28:21 -0700 (PDT)\r
+Received: from mail-fx0-f53.google.com (mail-fx0-f53.google.com\r
+ [209.85.161.53]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
+ (No client certificate requested)\r
+ by olra.theworths.org (Postfix) with ESMTPS id CA1E7431FB6\r
+ for <notmuch@notmuchmail.org>; Sat, 29 Oct 2011 13:28:20 -0700 (PDT)\r
+Received: by faai28 with SMTP id i28so5104411faa.26\r
+ for <notmuch@notmuchmail.org>; Sat, 29 Oct 2011 13:28:19 -0700 (PDT)\r
+Received: by 10.223.61.138 with SMTP id t10mr16144349fah.20.1319920099347;\r
+ Sat, 29 Oct 2011 13:28:19 -0700 (PDT)\r
+Received: from localhost (dsl-hkibrasgw4-fe5cdc00-23.dhcp.inet.fi.\r
+ [80.220.92.23])\r
+ by mx.google.com with ESMTPS id d21sm23544063fac.4.2011.10.29.13.28.16\r
+ (version=SSLv3 cipher=OTHER); Sat, 29 Oct 2011 13:28:18 -0700 (PDT)\r
+From: Jani Nikula <jani@nikula.org>\r
+To: Daniel Schoepe <daniel@schoepe.org>, notmuch@notmuchmail.org\r
+Subject: Re: [RFC PATCH 3/3] emacs: support limiting the number of messages\r
+ shown in search results\r
+In-Reply-To: <87pqhfk8vx.fsf@gilead.invalid>\r
+References: <cover.1319833617.git.jani@nikula.org>\r
+ <0290050284e4cb3a6f5ef0eb3582319f8d52ccf2.1319833617.git.jani@nikula.org>\r
+ <87pqhfk8vx.fsf@gilead.invalid>\r
+User-Agent: Notmuch/0.9-20-gc4362a8 (http://notmuchmail.org) Emacs/23.3.1\r
+ (i686-pc-linux-gnu)\r
+Date: Sat, 29 Oct 2011 23:28:14 +0300\r
+Message-ID: <8762j7r19d.fsf@nikula.org>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+Cc: amdragon@mit.edu\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Sat, 29 Oct 2011 20:28:21 -0000\r
+\r
+On Sat, 29 Oct 2011 19:25:22 +0200, Daniel Schoepe <daniel@schoepe.org> wrote:\r
+> On Fri, 28 Oct 2011 23:59:31 +0300, Jani Nikula <jani@nikula.org> wrote:\r
+> > Add support for limiting the maximum number of messages initially displayed\r
+> > in search results. When enabled, the search results will contain push\r
+> > buttons to double the number of messages displayed or to show unlimited\r
+> > messages.\r
+> \r
+> Nice patch, as it not only makes searches with a lot of results easier\r
+> to use on slower machines/hard drives, but I also find that seeing only\r
+> a few dozen threads in the buffer looks more "orderly" to me, compared\r
+> to a buffer with hundreds of lines.\r
+\r
+Thanks, I agree. Though having read your review below, it's not such a\r
+nice patch after all. :)\r
+\r
+Also, having chatted with amdragon on IRC, patches 1 and 2 should be\r
+thought out better. Perhaps changing the lib is not the way to go after\r
+all, even if that would help other lib users in paging the results. But\r
+the whole thing was a quick proof of concept (hence "RFC") to get to see\r
+how this patch would work out, and I'm glad you liked it.\r
+\r
+> A few comments about the code:\r
+> \r
+> > @@ -373,6 +381,7 @@ Complete list of currently available key bindings:\r
+> > (make-local-variable 'notmuch-search-oldest-first)\r
+> > (make-local-variable 'notmuch-search-target-thread)\r
+> > (make-local-variable 'notmuch-search-target-line)\r
+> > + (make-local-variable 'notmuch-search-maxitems)\r
+> > (set (make-local-variable 'notmuch-search-continuation) nil)\r
+> > (set (make-local-variable 'scroll-preserve-screen-position) t)\r
+> > (add-to-invisibility-spec 'notmuch-search)\r
+> > @@ -633,6 +642,8 @@ This function advances the next thread when finished."\r
+> > (insert "End of search results.")\r
+> > (if (not (= exit-status 0))\r
+> > (insert (format " (process returned %d)" exit-status)))\r
+> > + (if notmuch-search-maxitems\r
+> > + (notmuch-search-setup-buttons))\r
+> \r
+> As discussed on IRC, this causes `notmuch-search' to fail if the\r
+> maxitems argument is nil.\r
+\r
+And as you pointed out, the parameter is optional so it must accept nil.\r
+\r
+> > +(defun notmuch-search-setup-buttons ()\r
+> > + (widget-insert " ")\r
+> > + (widget-create 'push-button\r
+> > + :notify (lambda (&rest ignore)\r
+> > + (set 'notmuch-search-maxitems\r
+> > + (* 2 notmuch-search-maxitems))\r
+> > + (notmuch-search-refresh-view))\r
+> > + :help-echo "Double the number of messages shown"\r
+> > + "Show 2X messages")\r
+> > + (widget-insert " ")\r
+> > + (widget-create 'push-button\r
+> > + :notify (lambda (&rest ignore)\r
+> > + (set 'notmuch-search-maxitems 0)\r
+> > + (notmuch-search-refresh-view))\r
+> > + :help-echo "Show all search results"\r
+> > + "Show unlimited messages")\r
+> > + (widget-setup))\r
+> \r
+> I think these notify-actions should be separate functions to make it\r
+> easier to bind them to keys.\r
+\r
+It's obvious now that you say it! :)\r
+\r
+> > +\r
+> > (defcustom notmuch-poll-script ""\r
+> > "An external script to incorporate new mail into the notmuch database.\r
+> > \r
+> > @@ -997,7 +1030,7 @@ current search results AND the additional query string provided."\r
+> > query)))\r
+> > (notmuch-search (if (string= notmuch-search-query-string "*")\r
+> > grouped-query\r
+> > - (concat notmuch-search-query-string " and " grouped-query)) notmuch-search-oldest-first)))\r
+> > + (concat notmuch-search-query-string " and "\r
+> > grouped-query)) notmuch-search-oldest-first\r
+> > notmuch-search-maxitems)))\r
+> \r
+> This causes notmuch-search-filter to fail (repeatedly), since `notmuch-search'\r
+> expects a TARGET-THREAD (or nil) as its third parameter, but is given\r
+> `notmuch-search-maxitems' instead.\r
+\r
+Oh, yes, totally broken.\r
+\r
+> > \r
+> > (defun notmuch-search-filter-by-tag (tag)\r
+> > "Filter the current search results based on a single tag.\r
+> > @@ -1006,7 +1039,7 @@ Runs a new search matching only messages that match both the\r
+> > current search results AND that are tagged with the given tag."\r
+> > (interactive\r
+> > (list (notmuch-select-tag-with-completion "Filter by tag: ")))\r
+> > - (notmuch-search (concat notmuch-search-query-string " and tag:" tag) notmuch-search-oldest-first))\r
+> > + (notmuch-search (concat notmuch-search-query-string " and tag:"\r
+> > tag) notmuch-search-oldest-first notmuch-search-maxitems))\r
+> \r
+> Same here.\r
+\r
+Yup.\r
+\r
+Many thanks for the review. I'll fix these for myself so I might send a\r
+v2 just as well.\r
+\r
+\r
+BR,\r
+Jani.\r
projects / notmuch-archives.git / commitdiff