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