unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results
@ 2011-10-31 21:18 Jani Nikula
  2011-10-31 21:18 ` [RFC PATCH v2 1/3] lib: add function to get the number of threads matching a search Jani Nikula
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Jani Nikula @ 2011-10-31 21:18 UTC (permalink / raw)
  To: notmuch; +Cc: amdragon

Hi, this is an iteration of id:"cover.1319833617.git.jani@nikula.org" addressing
comments on the list and IRC. Main changes:

* Results are now limited based on threads (not messages) for thread and summary
  output. This is accomplished with a new lib function to count the number of
  threads in matching messages.

* cli part is now inspired by James Vasile's patch
  id:"8739gyw0zh.fsf@opensourcematters.org", with the additional ability to
  limit from the end of result set.

* Bugs reported by Daniel Schoepe fixed.

* Don't show buttons if everything is visible already.

I'm still marking it as RFC. It works for me, but patch 1 might be deemed
unacceptable.


BR,
Jani.


Jani Nikula (3):
  lib: add function to get the number of threads matching a search
  cli: add options --first and --maxitems to notmuch search
  emacs: support limiting the number of results shown in search results

 emacs/notmuch-hello.el |   17 ++++++++++--
 emacs/notmuch.el       |   53 ++++++++++++++++++++++++++++++++++---
 lib/notmuch.h          |   14 ++++++++++
 lib/query.cc           |   40 ++++++++++++++++++++++++++++
 notmuch-search.c       |   67 +++++++++++++++++++++++++++++++++++++++--------
 5 files changed, 171 insertions(+), 20 deletions(-)

-- 
1.7.5.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH v2 1/3] lib: add function to get the number of threads matching a search
  2011-10-31 21:18 [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results Jani Nikula
@ 2011-10-31 21:18 ` Jani Nikula
  2011-10-31 21:18 ` [RFC PATCH v2 2/3] cli: add options --first and --maxitems to notmuch search Jani Nikula
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2011-10-31 21:18 UTC (permalink / raw)
  To: notmuch; +Cc: amdragon

Add function notmuch_query_count_threads() to get the number of threads
matching a search. This is done by performing a search and figuring out the
number of unique thread IDs in the matching messages, a significantly
heavier operation than notmuch_query_count_messages().

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 lib/notmuch.h |   14 ++++++++++++++
 lib/query.cc  |   40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index c4330e4..662d594 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -609,6 +609,20 @@ notmuch_threads_destroy (notmuch_threads_t *threads);
 unsigned
 notmuch_query_count_messages (notmuch_query_t *query);
  
+/* Return the number of threads matching a search.
+ *
+ * This function performs a search and returns the number of unique thread IDs
+ * in the matching messages. This is the same as number of threads matching a
+ * search.
+ *
+ * Note that this is a fairly heavy operation, much more so than
+ * notmuch_query_count_messages().
+ *
+ * If an error occurs, this function may return 0.
+ */
+unsigned
+notmuch_query_count_threads (notmuch_query_t *query);
+
 /* Get the thread ID of 'thread'.
  *
  * The returned string belongs to 'thread' and as such, should not be
diff --git a/lib/query.cc b/lib/query.cc
index 6f02b04..2c0610a 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -457,3 +457,43 @@ notmuch_query_count_messages (notmuch_query_t *query)
 
     return count;
 }
+
+unsigned
+notmuch_query_count_threads (notmuch_query_t *query)
+{
+    notmuch_messages_t *messages;
+    GHashTable *hash;
+    unsigned int count;
+
+    messages = notmuch_query_search_messages (query);
+    if (messages == NULL)
+	return 0;
+
+    hash = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL);
+    if (hash == NULL) {
+	talloc_free (messages);
+	return 0;
+    }
+
+    while (notmuch_messages_valid (messages)) {
+	notmuch_message_t *message = notmuch_messages_get (messages);
+	const char *thread_id = notmuch_message_get_thread_id (message);
+	char *thread_id_copy = talloc_strdup(messages, thread_id);
+	if (unlikely (thread_id_copy == NULL)) {
+	    notmuch_message_destroy (message);
+	    count = 0;
+	    goto DONE;
+	}
+	g_hash_table_insert (hash, thread_id_copy, NULL);
+	notmuch_message_destroy (message);
+	notmuch_messages_move_to_next (messages);
+    }
+
+    count = g_hash_table_size(hash);
+
+  DONE:
+    g_hash_table_unref (hash);
+    talloc_free (messages);
+
+    return count;
+}
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC PATCH v2 2/3] cli: add options --first and --maxitems to notmuch search
  2011-10-31 21:18 [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results Jani Nikula
  2011-10-31 21:18 ` [RFC PATCH v2 1/3] lib: add function to get the number of threads matching a search Jani Nikula
@ 2011-10-31 21:18 ` Jani Nikula
  2011-10-31 21:18 ` [RFC PATCH v2 3/3] emacs: support limiting the number of results shown in search results Jani Nikula
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2011-10-31 21:18 UTC (permalink / raw)
  To: notmuch; +Cc: amdragon

Add options --first=[-]N and --maxitems=M to notmuch search to determine
the first result and maximum number of results to display.

Option --maxitems=M limits the maximum number of results to display to M.

Option --first=[-]N skips the first N results; with the leading '-' skip
until the Nth result from the end (showing a total of N results if within
bounds of the total number of results and not limited with --maxitems).

Note that --first with a negative N for thread or summary output requires
counting the number of matching threads in advance, which is a heavy
operation.

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 notmuch-search.c |   67 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 6f04d9a..ceef6ac 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -194,13 +194,20 @@ static int
 do_search_threads (const search_format_t *format,
 		   notmuch_query_t *query,
 		   notmuch_sort_t sort,
-		   output_t output)
+		   output_t output,
+		   int first,
+		   unsigned int maxitems)
 {
     notmuch_thread_t *thread;
     notmuch_threads_t *threads;
     notmuch_tags_t *tags;
     time_t date;
     int first_thread = 1;
+    unsigned int i, start;
+
+    if (first < 0)
+	first += notmuch_query_count_threads (query);
+    start = first < 0 ? 0 : first;
 
     threads = notmuch_query_search_threads (query);
     if (threads == NULL)
@@ -208,17 +215,23 @@ do_search_threads (const search_format_t *format,
 
     fputs (format->results_start, stdout);
 
-    for (;
-	 notmuch_threads_valid (threads);
-	 notmuch_threads_move_to_next (threads))
+    for (i = 0;
+	 notmuch_threads_valid (threads) &&
+	     (!maxitems || i < start + maxitems);
+	 notmuch_threads_move_to_next (threads), i++)
     {
 	int first_tag = 1;
 
+	thread = notmuch_threads_get (threads);
+
+	if (i < start) {
+	    notmuch_thread_destroy (thread);
+	    continue;
+	}
+
 	if (! first_thread)
 	    fputs (format->item_sep, stdout);
 
-	thread = notmuch_threads_get (threads);
-
 	if (output == OUTPUT_THREADS) {
 	    format->item_id (thread, "thread:",
 			     notmuch_thread_get_thread_id (thread));
@@ -271,12 +284,20 @@ do_search_threads (const search_format_t *format,
 static int
 do_search_messages (const search_format_t *format,
 		    notmuch_query_t *query,
-		    output_t output)
+		    output_t output,
+		    int first,
+		    unsigned int maxitems)
 {
     notmuch_message_t *message;
     notmuch_messages_t *messages;
     notmuch_filenames_t *filenames;
     int first_message = 1;
+    unsigned int i, start;
+
+    if (first < 0)
+	first += notmuch_query_count_messages(query);
+
+    start = first < 0 ? 0 : first;
 
     messages = notmuch_query_search_messages (query);
     if (messages == NULL)
@@ -284,10 +305,14 @@ do_search_messages (const search_format_t *format,
 
     fputs (format->results_start, stdout);
 
-    for (;
-	 notmuch_messages_valid (messages);
-	 notmuch_messages_move_to_next (messages))
+    for (i = 0;
+	 notmuch_messages_valid (messages) &&
+	     (!maxitems || i < start + maxitems);
+	 notmuch_messages_move_to_next (messages), i++)
     {
+	if (i < start)
+	    continue;
+
 	message = notmuch_messages_get (messages);
 
 	if (output == OUTPUT_FILES) {
@@ -394,6 +419,8 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
     const search_format_t *format = &format_text;
     int i, ret;
     output_t output = OUTPUT_SUMMARY;
+    unsigned int maxitems = 0;
+    int first = 0;
 
     argc--; argv++; /* skip subcommand argument */
 
@@ -412,6 +439,22 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 		fprintf (stderr, "Invalid value for --sort: %s\n", opt);
 		return 1;
 	    }
+	} else if (STRNCMP_LITERAL (argv[i], "--first=") == 0) {
+	    char *p;
+	    opt = argv[i] + sizeof ("--first=") - 1;
+	    first = strtol(opt, &p, 10);
+	    if (*opt == '\0' || p == opt || *p != '\0') {
+		fprintf (stderr, "Invalid value for --first: %s\n", opt);
+		return 1;
+	    }
+	} else if (STRNCMP_LITERAL (argv[i], "--maxitems=") == 0) {
+	    char *p;
+	    opt = argv[i] + sizeof ("--maxitems=") - 1;
+	    maxitems = strtoul(opt, &p, 10);
+	    if (*opt == '\0' || p == opt || *p != '\0') {
+		fprintf (stderr, "Invalid value for --maxitems: %s\n", opt);
+		return 1;
+	    }
 	} else if (STRNCMP_LITERAL (argv[i], "--format=") == 0) {
 	    opt = argv[i] + sizeof ("--format=") - 1;
 	    if (strcmp (opt, "text") == 0) {
@@ -478,11 +521,11 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
     default:
     case OUTPUT_SUMMARY:
     case OUTPUT_THREADS:
-	ret = do_search_threads (format, query, sort, output);
+	ret = do_search_threads (format, query, sort, output, first, maxitems);
 	break;
     case OUTPUT_MESSAGES:
     case OUTPUT_FILES:
-	ret = do_search_messages (format, query, output);
+	ret = do_search_messages (format, query, output, first, maxitems);
 	break;
     case OUTPUT_TAGS:
 	ret = do_search_tags (notmuch, format, query);
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC PATCH v2 3/3] emacs: support limiting the number of results shown in search results
  2011-10-31 21:18 [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results Jani Nikula
  2011-10-31 21:18 ` [RFC PATCH v2 1/3] lib: add function to get the number of threads matching a search Jani Nikula
  2011-10-31 21:18 ` [RFC PATCH v2 2/3] cli: add options --first and --maxitems to notmuch search Jani Nikula
@ 2011-10-31 21:18 ` Jani Nikula
  2011-11-04 20:54   ` Austin Clements
  2011-10-31 21:44 ` [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages " Jameson Graef Rollins
  2011-11-01 12:30 ` David Bremner
  4 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2011-10-31 21:18 UTC (permalink / raw)
  To: notmuch; +Cc: amdragon

Add support for limiting the maximum number of results initially displayed
in search results. When enabled, the search results will contain push
buttons to double the number of results displayed or to show unlimited
results.

The approach is inspired by vc-print-log in Emacs vc.el.

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 emacs/notmuch-hello.el |   17 ++++++++++++--
 emacs/notmuch.el       |   53 +++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 62 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 65fde75..4ba13e3 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -26,7 +26,7 @@
 (require 'notmuch-lib)
 (require 'notmuch-mua)
 
-(declare-function notmuch-search "notmuch" (query &optional oldest-first target-thread target-line continuation))
+(declare-function notmuch-search "notmuch" (query &optional oldest-first maxitems target-thread target-line continuation))
 (declare-function notmuch-poll "notmuch" ())
 
 (defvar notmuch-hello-search-bar-marker nil
@@ -37,6 +37,17 @@
   :type 'integer
   :group 'notmuch)
 
+(defcustom notmuch-search-maxitems 0
+  "The maximum number of results to show in search results.
+
+This variables controls the maximum number of results to
+initially show in search results. Set to 0 to not limit the
+number of results. If non-zero, the search results will contain
+push buttons to double the number (can be repeated) or show
+unlimited number of results."
+  :type 'integer
+  :group 'notmuch)
+
 (defcustom notmuch-show-empty-saved-searches nil
   "Should saved searches with no messages be listed?"
   :type 'boolean
@@ -151,7 +162,7 @@ Typically \",\" in the US and UK and \".\" in Europe."
 (defun notmuch-hello-search (search)
   (let ((search (notmuch-hello-trim search)))
     (notmuch-hello-remember-search search)
-    (notmuch-search search notmuch-search-oldest-first nil nil #'notmuch-hello-search-continuation)))
+    (notmuch-search search notmuch-search-oldest-first notmuch-search-maxitems nil nil #'notmuch-hello-search-continuation)))
 
 (defun notmuch-hello-add-saved-search (widget)
   (interactive)
@@ -200,7 +211,7 @@ diagonal."
 (defun notmuch-hello-widget-search (widget &rest ignore)
   (notmuch-search (widget-get widget
 			      :notmuch-search-terms)
-		  notmuch-search-oldest-first
+		  notmuch-search-oldest-first notmuch-search-maxitems
 		  nil nil #'notmuch-hello-search-continuation))
 
 (defun notmuch-saved-search-count (search)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index f11ec24..741ebe1 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -193,6 +193,7 @@ For a mouse binding, return nil."
 
 (defvar notmuch-search-mode-map
   (let ((map (make-sparse-keymap)))
+    (set-keymap-parent map widget-keymap)
     (define-key map "?" 'notmuch-help)
     (define-key map "q" 'notmuch-search-quit)
     (define-key map "x" 'notmuch-search-quit)
@@ -217,7 +218,13 @@ For a mouse binding, return nil."
     (define-key map "a" 'notmuch-search-archive-thread)
     (define-key map "-" 'notmuch-search-remove-tag)
     (define-key map "+" 'notmuch-search-add-tag)
-    (define-key map (kbd "RET") 'notmuch-search-show-thread)
+    ; Some hackery to allow RET both on buttons and messages. There's probably a
+    ; better way to do this...
+    (define-key map (kbd "RET") '(lambda (pos)
+				   (interactive "@d")
+				   (if (get-char-property pos 'button)
+				       (widget-button-press pos)
+				     (notmuch-search-show-thread))))
     (define-key map (kbd "M-RET") 'notmuch-search-show-thread-crypto-switch)
     map)
   "Keymap for \"notmuch search\" buffers.")
@@ -239,6 +246,7 @@ For a mouse binding, return nil."
 (defvar notmuch-search-target-thread)
 (defvar notmuch-search-target-line)
 (defvar notmuch-search-continuation)
+(defvar notmuch-search-maxitems)
 
 (defvar notmuch-search-disjunctive-regexp      "\\<[oO][rR]\\>")
 
@@ -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,11 @@ 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 (and notmuch-search-maxitems
+				 (< 0 notmuch-search-maxitems)
+				 (< notmuch-search-maxitems
+				    (count-lines (point-min) (point-max))))
+			    (notmuch-search-setup-buttons))
 			(insert "\n")
 			(if (and atbob
 				 (not (string= notmuch-search-target-thread "found")))
@@ -883,7 +897,7 @@ characters as well as `_.+-'.
 	  )))
 
 ;;;###autoload
-(defun notmuch-search (query &optional oldest-first target-thread target-line continuation)
+(defun notmuch-search (query &optional oldest-first maxitems target-thread target-line continuation)
   "Run \"notmuch search\" with the given query string and display results.
 
 The optional parameters are used as follows:
@@ -899,6 +913,7 @@ The optional parameters are used as follows:
     (notmuch-search-mode)
     (set 'notmuch-search-query-string query)
     (set 'notmuch-search-oldest-first oldest-first)
+    (set 'notmuch-search-maxitems maxitems)
     (set 'notmuch-search-target-thread target-thread)
     (set 'notmuch-search-target-line target-line)
     (set 'notmuch-search-continuation continuation)
@@ -916,6 +931,11 @@ The optional parameters are used as follows:
 		     (if oldest-first
 			 "--sort=oldest-first"
 		       "--sort=newest-first")
+		     (if (and maxitems (< 0 maxitems))
+			 (if oldest-first
+			     (format "--first=-%d" maxitems)
+			   (format "--maxitems=%d" maxitems))
+		       "")
 		     query)))
 	  (set-process-sentinel proc 'notmuch-search-process-sentinel)
 	  (set-process-filter proc 'notmuch-search-process-filter))))
@@ -932,13 +952,36 @@ same relative position within the new buffer."
   (interactive)
   (let ((target-line (line-number-at-pos))
 	(oldest-first notmuch-search-oldest-first)
+	(maxitems notmuch-search-maxitems)
 	(target-thread (notmuch-search-find-thread-id))
 	(query notmuch-search-query-string)
 	(continuation notmuch-search-continuation))
     (notmuch-kill-this-buffer)
-    (notmuch-search query oldest-first target-thread target-line continuation)
+    (notmuch-search query oldest-first maxitems target-thread target-line continuation)
     (goto-char (point-min))))
 
+(defun notmuch-search-double-results (&rest ignore)
+  (if notmuch-search-maxitems
+      (set 'notmuch-search-maxitems (* 2 notmuch-search-maxitems)))
+  (notmuch-search-refresh-view))
+
+(defun notmuch-search-unlimited-results (&rest ignore)
+  (set 'notmuch-search-maxitems nil)
+  (notmuch-search-refresh-view))
+
+(defun notmuch-search-setup-buttons ()
+  (widget-insert "    ")
+  (widget-create 'push-button
+		 :notify 'notmuch-search-double-results
+		 :help-echo "Double the number of results shown"
+		 "Show 2X results")
+  (widget-insert "    ")
+  (widget-create 'push-button
+		 :notify 'notmuch-search-unlimited-results
+		 :help-echo "Show all search results"
+		 "Show unlimited results")
+  (widget-setup))
+
 (defcustom notmuch-poll-script ""
   "An external script to incorporate new mail into the notmuch database.
 
@@ -997,7 +1040,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)))
 
 (defun notmuch-search-filter-by-tag (tag)
   "Filter the current search results based on a single tag.
@@ -1006,7 +1049,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))
 
 ;;;###autoload
 (defun notmuch ()
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results
  2011-10-31 21:18 [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results Jani Nikula
                   ` (2 preceding siblings ...)
  2011-10-31 21:18 ` [RFC PATCH v2 3/3] emacs: support limiting the number of results shown in search results Jani Nikula
@ 2011-10-31 21:44 ` Jameson Graef Rollins
  2011-10-31 22:07   ` Jani Nikula
  2011-11-01 12:30 ` David Bremner
  4 siblings, 1 reply; 12+ messages in thread
From: Jameson Graef Rollins @ 2011-10-31 21:44 UTC (permalink / raw)
  To: Jani Nikula, notmuch; +Cc: amdragon

[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]

On Mon, 31 Oct 2011 23:18:07 +0200, Jani Nikula <jani@nikula.org> wrote:
> Hi, this is an iteration of id:"cover.1319833617.git.jani@nikula.org" addressing
> comments on the list and IRC. Main changes:
> 
> * Results are now limited based on threads (not messages) for thread and summary
>   output. This is accomplished with a new lib function to count the number of
>   threads in matching messages.
> 
> * cli part is now inspired by James Vasile's patch
>   id:"8739gyw0zh.fsf@opensourcematters.org", with the additional ability to
>   limit from the end of result set.
> 
> * Bugs reported by Daniel Schoepe fixed.
> 
> * Don't show buttons if everything is visible already.
> 
> I'm still marking it as RFC. It works for me, but patch 1 might be deemed
> unacceptable.

Hi, Jani.  Thanks for working on this.  This should also be valuable for
vim users.

In order to push forward with this, though, I think we really need to
have a complete unit test for this new functionality.  We usually like
to see units tests that describe and then test for the new functionality
you wish to add, followed by the patches that provide the new
functionality.  Lots of good tests for new functionality being proposed
here shouldn't be too difficult to work out ahead of time.

For instance, here's an example of a test that I would like to see:

test_begin_subtest "maxitems does the right thing"
notmuch search tag:foo | head -n 20 >expected
notmuch search --maxitems=20 tag:foo >output
test_expect_equal_file expected output

test_begin_subtest "concatenation of limited searches does the right thing"
notmuch search tag:foo | head -n 20 >expected
notmuch search --maxitems=10 tag:foo >output
notmuch search --maxitems=10 --first=10 tag:foo >>output
test_expect_equal_file expected output


jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results
  2011-10-31 21:44 ` [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages " Jameson Graef Rollins
@ 2011-10-31 22:07   ` Jani Nikula
  2011-10-31 22:19     ` Jameson Graef Rollins
  2011-11-04 20:21     ` Austin Clements
  0 siblings, 2 replies; 12+ messages in thread
From: Jani Nikula @ 2011-10-31 22:07 UTC (permalink / raw)
  To: Jameson Graef Rollins, notmuch; +Cc: amdragon

On Mon, 31 Oct 2011 14:44:29 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Hi, Jani.  Thanks for working on this.  This should also be valuable for
> vim users.

Thanks for your interest! :)

> In order to push forward with this, though, I think we really need to
> have a complete unit test for this new functionality.  We usually like
> to see units tests that describe and then test for the new functionality
> you wish to add, followed by the patches that provide the new
> functionality.  Lots of good tests for new functionality being proposed
> here shouldn't be too difficult to work out ahead of time.

Right. I'd just like to make sure the approach I've taken (particularly
patch 1 in the set as it touches the lib) is acceptable before spending
time on testing and documentation etc. Indeed patches 1 and 2 changed
fundamentally between v1 and v2 after some chats on IRC. If the comments
now are favourable, I'll write the tests and documentation. (Though I
guess I have to admit the tests would've been beneficial to me already
now...)

BR,
Jani.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results
  2011-10-31 22:07   ` Jani Nikula
@ 2011-10-31 22:19     ` Jameson Graef Rollins
  2011-11-04 20:21     ` Austin Clements
  1 sibling, 0 replies; 12+ messages in thread
From: Jameson Graef Rollins @ 2011-10-31 22:19 UTC (permalink / raw)
  To: Jani Nikula, notmuch; +Cc: amdragon

[-- Attachment #1: Type: text/plain, Size: 1081 bytes --]

On Tue, 01 Nov 2011 00:07:59 +0200, Jani Nikula <jani@nikula.org> wrote:
> On Mon, 31 Oct 2011 14:44:29 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Right. I'd just like to make sure the approach I've taken (particularly
> patch 1 in the set as it touches the lib) is acceptable before spending
> time on testing and documentation etc. Indeed patches 1 and 2 changed
> fundamentally between v1 and v2 after some chats on IRC. If the comments
> now are favourable, I'll write the tests and documentation. (Though I
> guess I have to admit the tests would've been beneficial to me already
> now...)

I would say that since this is functionality that we do eventually want,
a known_broken test suite that tests for the functionality we want to
see would be very useful right from the start, even before any patches
to supply the functionality are made.  With the tests in hand we can
iterate over exactly what we want the ui to provide, and then worry
about the specific implementation.

The documentation can come later with the actual feature patches.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results
  2011-10-31 21:18 [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results Jani Nikula
                   ` (3 preceding siblings ...)
  2011-10-31 21:44 ` [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages " Jameson Graef Rollins
@ 2011-11-01 12:30 ` David Bremner
  2011-11-02  9:27   ` Jani Nikula
  4 siblings, 1 reply; 12+ messages in thread
From: David Bremner @ 2011-11-01 12:30 UTC (permalink / raw)
  To: Jani Nikula, notmuch

On Mon, 31 Oct 2011 23:18:07 +0200, Jani Nikula <jani@nikula.org> wrote:
> 
> I'm still marking it as RFC. It works for me, but patch 1 might be deemed
> unacceptable.
> 

Hi Jani;

Is just because it add a function to the library that you think this
might be problematic?  I don't think we are super-dogmatic about the
library never growing.  When notmuch started, there were no bindings, so
in retrospect maybe more functionality went into the CLI than might
happen if we started from scratch. If I remember Carl's statement
correctly, one rule is that stuff in the library should not require
configuration.

Just my personal view,

David

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results
  2011-11-01 12:30 ` David Bremner
@ 2011-11-02  9:27   ` Jani Nikula
  0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2011-11-02  9:27 UTC (permalink / raw)
  To: David Bremner, notmuch

On Tue, 01 Nov 2011 09:30:56 -0300, David Bremner <david@tethera.net> wrote:
> Is just because it add a function to the library that you think this
> might be problematic?  I don't think we are super-dogmatic about the
> library never growing.  When notmuch started, there were no bindings, so
> in retrospect maybe more functionality went into the CLI than might
> happen if we started from scratch. If I remember Carl's statement
> correctly, one rule is that stuff in the library should not require
> configuration.

Hi, thanks, that's encouraging. It's mostly that, based on past
experience, I'm hesitant about extending a library interface. Once you
get users, you have to live with it. So you want to get it right.

Having said that, I think in this v2 of the set, the library interface
for notmuch_query_count_threads() is sane and complimentary to the
existing notmuch_query_count_messages(). (Also quoting IRC: "<amdragon>
j4ni: Yes, the interface seems totally reasonable.") Later on, someone
might come up with a better implementation, but for now it's already
much better than having to iterate and construct the threads.

I'll prepare a v3 with some tests and polish.

BR,
Jani.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results
  2011-10-31 22:07   ` Jani Nikula
  2011-10-31 22:19     ` Jameson Graef Rollins
@ 2011-11-04 20:21     ` Austin Clements
  1 sibling, 0 replies; 12+ messages in thread
From: Austin Clements @ 2011-11-04 20:21 UTC (permalink / raw)
  To: Jani Nikula; +Cc: notmuch

On Mon, Oct 31, 2011 at 6:07 PM, Jani Nikula <jani@nikula.org> wrote:
> On Mon, 31 Oct 2011 14:44:29 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
>> In order to push forward with this, though, I think we really need to
>> have a complete unit test for this new functionality.  We usually like
>> to see units tests that describe and then test for the new functionality
>> you wish to add, followed by the patches that provide the new
>> functionality.  Lots of good tests for new functionality being proposed
>> here shouldn't be too difficult to work out ahead of time.
>
> Right. I'd just like to make sure the approach I've taken (particularly
> patch 1 in the set as it touches the lib) is acceptable before spending
> time on testing and documentation etc. Indeed patches 1 and 2 changed
> fundamentally between v1 and v2 after some chats on IRC. If the comments
> now are favourable, I'll write the tests and documentation. (Though I
> guess I have to admit the tests would've been beneficial to me already
> now...)

The library interface looks perfectly reasonable and consistent to me.
 My only concern would be that there's no way to return errors from
notmuch_query_count_threads, but notmuch_query_count_messages has
exactly the same problem.

Other than that, you missed a few spaces before parentheses.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH v2 3/3] emacs: support limiting the number of results shown in search results
  2011-10-31 21:18 ` [RFC PATCH v2 3/3] emacs: support limiting the number of results shown in search results Jani Nikula
@ 2011-11-04 20:54   ` Austin Clements
  2011-11-06 20:57     ` Jani Nikula
  0 siblings, 1 reply; 12+ messages in thread
From: Austin Clements @ 2011-11-04 20:54 UTC (permalink / raw)
  To: Jani Nikula; +Cc: notmuch

Quoth Jani Nikula on Oct 31 at 11:18 pm:
> Add support for limiting the maximum number of results initially displayed
> in search results. When enabled, the search results will contain push
> buttons to double the number of results displayed or to show unlimited
> results.
> 
> The approach is inspired by vc-print-log in Emacs vc.el.
> 
> Signed-off-by: Jani Nikula <jani@nikula.org>
> ---
>  emacs/notmuch-hello.el |   17 ++++++++++++--
>  emacs/notmuch.el       |   53 +++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 62 insertions(+), 8 deletions(-)
> 
> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> index 65fde75..4ba13e3 100644
> --- a/emacs/notmuch-hello.el
> +++ b/emacs/notmuch-hello.el
> @@ -26,7 +26,7 @@
>  (require 'notmuch-lib)
>  (require 'notmuch-mua)
>  
> -(declare-function notmuch-search "notmuch" (query &optional oldest-first target-thread target-line continuation))
> +(declare-function notmuch-search "notmuch" (query &optional oldest-first maxitems target-thread target-line continuation))
>  (declare-function notmuch-poll "notmuch" ())
>  
>  (defvar notmuch-hello-search-bar-marker nil
> @@ -37,6 +37,17 @@
>    :type 'integer
>    :group 'notmuch)
>  
> +(defcustom notmuch-search-maxitems 0
> +  "The maximum number of results to show in search results.
> +
> +This variables controls the maximum number of results to
> +initially show in search results. Set to 0 to not limit the
> +number of results. If non-zero, the search results will contain
> +push buttons to double the number (can be repeated) or show
> +unlimited number of results."
> +  :type 'integer
> +  :group 'notmuch)
> +

It would be more lispy to use nil to indicate no limit, rather than 0.
The type could be something like
  (choice (const   :tag "Unlimited" nil)
          (integer :tag "Limit"))

>  (defcustom notmuch-show-empty-saved-searches nil
>    "Should saved searches with no messages be listed?"
>    :type 'boolean
> @@ -151,7 +162,7 @@ Typically \",\" in the US and UK and \".\" in Europe."
>  (defun notmuch-hello-search (search)
>    (let ((search (notmuch-hello-trim search)))
>      (notmuch-hello-remember-search search)
> -    (notmuch-search search notmuch-search-oldest-first nil nil #'notmuch-hello-search-continuation)))
> +    (notmuch-search search notmuch-search-oldest-first notmuch-search-maxitems nil nil #'notmuch-hello-search-continuation)))

It might make more sense of notmuch-search to use the value of
notmuch-search-maxitems directly, rather than feeding it in
everywhere, especially since it's buffer-local.  If there's really a
place that needs to override it, it would be easy to let-bind the
variable there.

>  
>  (defun notmuch-hello-add-saved-search (widget)
>    (interactive)
> @@ -200,7 +211,7 @@ diagonal."
>  (defun notmuch-hello-widget-search (widget &rest ignore)
>    (notmuch-search (widget-get widget
>  			      :notmuch-search-terms)
> -		  notmuch-search-oldest-first
> +		  notmuch-search-oldest-first notmuch-search-maxitems
>  		  nil nil #'notmuch-hello-search-continuation))
>  
>  (defun notmuch-saved-search-count (search)
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index f11ec24..741ebe1 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -193,6 +193,7 @@ For a mouse binding, return nil."
>  
>  (defvar notmuch-search-mode-map
>    (let ((map (make-sparse-keymap)))
> +    (set-keymap-parent map widget-keymap)
>      (define-key map "?" 'notmuch-help)
>      (define-key map "q" 'notmuch-search-quit)
>      (define-key map "x" 'notmuch-search-quit)
> @@ -217,7 +218,13 @@ For a mouse binding, return nil."
>      (define-key map "a" 'notmuch-search-archive-thread)
>      (define-key map "-" 'notmuch-search-remove-tag)
>      (define-key map "+" 'notmuch-search-add-tag)
> -    (define-key map (kbd "RET") 'notmuch-search-show-thread)
> +    ; Some hackery to allow RET both on buttons and messages. There's probably a
> +    ; better way to do this...
> +    (define-key map (kbd "RET") '(lambda (pos)
> +				   (interactive "@d")
> +				   (if (get-char-property pos 'button)
> +				       (widget-button-press pos)
> +				     (notmuch-search-show-thread))))

Huh.  I thought widgets automatically had local keymaps, but maybe
that's only buttons (insert-button type buttons, that is).  A local
keymap is probably the right way to do this regardless.  Or use
buttons instead of widgets.

>      (define-key map (kbd "M-RET") 'notmuch-search-show-thread-crypto-switch)
>      map)
>    "Keymap for \"notmuch search\" buffers.")
> @@ -239,6 +246,7 @@ For a mouse binding, return nil."
>  (defvar notmuch-search-target-thread)
>  (defvar notmuch-search-target-line)
>  (defvar notmuch-search-continuation)
> +(defvar notmuch-search-maxitems)
>  
>  (defvar notmuch-search-disjunctive-regexp      "\\<[oO][rR]\\>")
>  
> @@ -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,11 @@ 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 (and notmuch-search-maxitems
> +				 (< 0 notmuch-search-maxitems)
> +				 (< notmuch-search-maxitems
> +				    (count-lines (point-min) (point-max))))
> +			    (notmuch-search-setup-buttons))
>  			(insert "\n")
>  			(if (and atbob
>  				 (not (string= notmuch-search-target-thread "found")))
> @@ -883,7 +897,7 @@ characters as well as `_.+-'.
>  	  )))
>  
>  ;;;###autoload
> -(defun notmuch-search (query &optional oldest-first target-thread target-line continuation)
> +(defun notmuch-search (query &optional oldest-first maxitems target-thread target-line continuation)
>    "Run \"notmuch search\" with the given query string and display results.
>  
>  The optional parameters are used as follows:
> @@ -899,6 +913,7 @@ The optional parameters are used as follows:
>      (notmuch-search-mode)
>      (set 'notmuch-search-query-string query)
>      (set 'notmuch-search-oldest-first oldest-first)
> +    (set 'notmuch-search-maxitems maxitems)
>      (set 'notmuch-search-target-thread target-thread)
>      (set 'notmuch-search-target-line target-line)
>      (set 'notmuch-search-continuation continuation)
> @@ -916,6 +931,11 @@ The optional parameters are used as follows:
>  		     (if oldest-first
>  			 "--sort=oldest-first"
>  		       "--sort=newest-first")
> +		     (if (and maxitems (< 0 maxitems))
> +			 (if oldest-first
> +			     (format "--first=-%d" maxitems)
> +			   (format "--maxitems=%d" maxitems))
> +		       "")
>  		     query)))
>  	  (set-process-sentinel proc 'notmuch-search-process-sentinel)
>  	  (set-process-filter proc 'notmuch-search-process-filter))))
> @@ -932,13 +952,36 @@ same relative position within the new buffer."
>    (interactive)
>    (let ((target-line (line-number-at-pos))
>  	(oldest-first notmuch-search-oldest-first)
> +	(maxitems notmuch-search-maxitems)
>  	(target-thread (notmuch-search-find-thread-id))
>  	(query notmuch-search-query-string)
>  	(continuation notmuch-search-continuation))
>      (notmuch-kill-this-buffer)
> -    (notmuch-search query oldest-first target-thread target-line continuation)
> +    (notmuch-search query oldest-first maxitems target-thread target-line continuation)
>      (goto-char (point-min))))
>  
> +(defun notmuch-search-double-results (&rest ignore)
> +  (if notmuch-search-maxitems
> +      (set 'notmuch-search-maxitems (* 2 notmuch-search-maxitems)))
> +  (notmuch-search-refresh-view))
> +
> +(defun notmuch-search-unlimited-results (&rest ignore)
> +  (set 'notmuch-search-maxitems nil)
> +  (notmuch-search-refresh-view))

I would recommend setq instead of set, except in the weird local
places where `set' is clearly the prevailing style.

> +
> +(defun notmuch-search-setup-buttons ()
> +  (widget-insert "    ")
> +  (widget-create 'push-button
> +		 :notify 'notmuch-search-double-results
> +		 :help-echo "Double the number of results shown"
> +		 "Show 2X results")
> +  (widget-insert "    ")
> +  (widget-create 'push-button
> +		 :notify 'notmuch-search-unlimited-results
> +		 :help-echo "Show all search results"
> +		 "Show unlimited results")
> +  (widget-setup))
> +

If you're limiting from the bottom, would it make more sense for these
widgets to be placed at the top of the buffer, since that's the
direction they'll expand the results?

>  (defcustom notmuch-poll-script ""
>    "An external script to incorporate new mail into the notmuch database.
>  
> @@ -997,7 +1040,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)))
>  
>  (defun notmuch-search-filter-by-tag (tag)
>    "Filter the current search results based on a single tag.
> @@ -1006,7 +1049,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))
>  
>  ;;;###autoload
>  (defun notmuch ()

-- 
Austin Clements                                      MIT/'06/PhD/CSAIL
amdragon@mit.edu                           http://web.mit.edu/amdragon
       Somewhere in the dream we call reality you will find me,
              searching for the reality we call dreams.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH v2 3/3] emacs: support limiting the number of results shown in search results
  2011-11-04 20:54   ` Austin Clements
@ 2011-11-06 20:57     ` Jani Nikula
  0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2011-11-06 20:57 UTC (permalink / raw)
  To: Austin Clements; +Cc: notmuch


Hi Austin, and many thanks for your comments. Before hacking on notmuch
my lisp was mostly limited to ~/.emacs, and unfortunately it shows...

On Fri, 4 Nov 2011 16:54:15 -0400, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Jani Nikula on Oct 31 at 11:18 pm:
> > Add support for limiting the maximum number of results initially displayed
> > in search results. When enabled, the search results will contain push
> > buttons to double the number of results displayed or to show unlimited
> > results.
> > 
> > The approach is inspired by vc-print-log in Emacs vc.el.
> > 
> > Signed-off-by: Jani Nikula <jani@nikula.org>
> > ---
> >  emacs/notmuch-hello.el |   17 ++++++++++++--
> >  emacs/notmuch.el       |   53 +++++++++++++++++++++++++++++++++++++++++++----
> >  2 files changed, 62 insertions(+), 8 deletions(-)
> > 
> > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> > index 65fde75..4ba13e3 100644
> > --- a/emacs/notmuch-hello.el
> > +++ b/emacs/notmuch-hello.el
> > @@ -26,7 +26,7 @@
> >  (require 'notmuch-lib)
> >  (require 'notmuch-mua)
> >  
> > -(declare-function notmuch-search "notmuch" (query &optional oldest-first target-thread target-line continuation))
> > +(declare-function notmuch-search "notmuch" (query &optional oldest-first maxitems target-thread target-line continuation))
> >  (declare-function notmuch-poll "notmuch" ())
> >  
> >  (defvar notmuch-hello-search-bar-marker nil
> > @@ -37,6 +37,17 @@
> >    :type 'integer
> >    :group 'notmuch)
> >  
> > +(defcustom notmuch-search-maxitems 0
> > +  "The maximum number of results to show in search results.
> > +
> > +This variables controls the maximum number of results to
> > +initially show in search results. Set to 0 to not limit the
> > +number of results. If non-zero, the search results will contain
> > +push buttons to double the number (can be repeated) or show
> > +unlimited number of results."
> > +  :type 'integer
> > +  :group 'notmuch)
> > +
> 
> It would be more lispy to use nil to indicate no limit, rather than 0.
> The type could be something like
>   (choice (const   :tag "Unlimited" nil)
>           (integer :tag "Limit"))

Definitely better.

> >  (defcustom notmuch-show-empty-saved-searches nil
> >    "Should saved searches with no messages be listed?"
> >    :type 'boolean
> > @@ -151,7 +162,7 @@ Typically \",\" in the US and UK and \".\" in Europe."
> >  (defun notmuch-hello-search (search)
> >    (let ((search (notmuch-hello-trim search)))
> >      (notmuch-hello-remember-search search)
> > -    (notmuch-search search notmuch-search-oldest-first nil nil #'notmuch-hello-search-continuation)))
> > +    (notmuch-search search notmuch-search-oldest-first notmuch-search-maxitems nil nil #'notmuch-hello-search-continuation)))
> 
> It might make more sense of notmuch-search to use the value of
> notmuch-search-maxitems directly, rather than feeding it in
> everywhere, especially since it's buffer-local.  If there's really a
> place that needs to override it, it would be easy to let-bind the
> variable there.

The only problem with that is that doubling the number of results is
done using notmuch-search-refresh-view, which kills the buffer and
performs a new search, which creates a new buffer. That loses the
doubled buffer-local value, doesn't it?

The Right Thing might be to use --first and --maxitems to only fetch the
missing results while preserving the old, but I was hoping that could be
a later improvement. It's not that trivial if new messages have been
added to the db in between.

> >  
> >  (defun notmuch-hello-add-saved-search (widget)
> >    (interactive)
> > @@ -200,7 +211,7 @@ diagonal."
> >  (defun notmuch-hello-widget-search (widget &rest ignore)
> >    (notmuch-search (widget-get widget
> >  			      :notmuch-search-terms)
> > -		  notmuch-search-oldest-first
> > +		  notmuch-search-oldest-first notmuch-search-maxitems
> >  		  nil nil #'notmuch-hello-search-continuation))
> >  
> >  (defun notmuch-saved-search-count (search)
> > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> > index f11ec24..741ebe1 100644
> > --- a/emacs/notmuch.el
> > +++ b/emacs/notmuch.el
> > @@ -193,6 +193,7 @@ For a mouse binding, return nil."
> >  
> >  (defvar notmuch-search-mode-map
> >    (let ((map (make-sparse-keymap)))
> > +    (set-keymap-parent map widget-keymap)
> >      (define-key map "?" 'notmuch-help)
> >      (define-key map "q" 'notmuch-search-quit)
> >      (define-key map "x" 'notmuch-search-quit)
> > @@ -217,7 +218,13 @@ For a mouse binding, return nil."
> >      (define-key map "a" 'notmuch-search-archive-thread)
> >      (define-key map "-" 'notmuch-search-remove-tag)
> >      (define-key map "+" 'notmuch-search-add-tag)
> > -    (define-key map (kbd "RET") 'notmuch-search-show-thread)
> > +    ; Some hackery to allow RET both on buttons and messages. There's probably a
> > +    ; better way to do this...
> > +    (define-key map (kbd "RET") '(lambda (pos)
> > +				   (interactive "@d")
> > +				   (if (get-char-property pos 'button)
> > +				       (widget-button-press pos)
> > +				     (notmuch-search-show-thread))))
> 
> Huh.  I thought widgets automatically had local keymaps, but maybe
> that's only buttons (insert-button type buttons, that is).  A local
> keymap is probably the right way to do this regardless.  Or use
> buttons instead of widgets.

I'll look into that, thanks.

> >      (define-key map (kbd "M-RET") 'notmuch-search-show-thread-crypto-switch)
> >      map)
> >    "Keymap for \"notmuch search\" buffers.")
> > @@ -239,6 +246,7 @@ For a mouse binding, return nil."
> >  (defvar notmuch-search-target-thread)
> >  (defvar notmuch-search-target-line)
> >  (defvar notmuch-search-continuation)
> > +(defvar notmuch-search-maxitems)
> >  
> >  (defvar notmuch-search-disjunctive-regexp      "\\<[oO][rR]\\>")
> >  
> > @@ -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,11 @@ 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 (and notmuch-search-maxitems
> > +				 (< 0 notmuch-search-maxitems)
> > +				 (< notmuch-search-maxitems
> > +				    (count-lines (point-min) (point-max))))
> > +			    (notmuch-search-setup-buttons))
> >  			(insert "\n")
> >  			(if (and atbob
> >  				 (not (string= notmuch-search-target-thread "found")))
> > @@ -883,7 +897,7 @@ characters as well as `_.+-'.
> >  	  )))
> >  
> >  ;;;###autoload
> > -(defun notmuch-search (query &optional oldest-first target-thread target-line continuation)
> > +(defun notmuch-search (query &optional oldest-first maxitems target-thread target-line continuation)
> >    "Run \"notmuch search\" with the given query string and display results.
> >  
> >  The optional parameters are used as follows:
> > @@ -899,6 +913,7 @@ The optional parameters are used as follows:
> >      (notmuch-search-mode)
> >      (set 'notmuch-search-query-string query)
> >      (set 'notmuch-search-oldest-first oldest-first)
> > +    (set 'notmuch-search-maxitems maxitems)
> >      (set 'notmuch-search-target-thread target-thread)
> >      (set 'notmuch-search-target-line target-line)
> >      (set 'notmuch-search-continuation continuation)
> > @@ -916,6 +931,11 @@ The optional parameters are used as follows:
> >  		     (if oldest-first
> >  			 "--sort=oldest-first"
> >  		       "--sort=newest-first")
> > +		     (if (and maxitems (< 0 maxitems))
> > +			 (if oldest-first
> > +			     (format "--first=-%d" maxitems)
> > +			   (format "--maxitems=%d" maxitems))
> > +		       "")
> >  		     query)))
> >  	  (set-process-sentinel proc 'notmuch-search-process-sentinel)
> >  	  (set-process-filter proc 'notmuch-search-process-filter))))
> > @@ -932,13 +952,36 @@ same relative position within the new buffer."
> >    (interactive)
> >    (let ((target-line (line-number-at-pos))
> >  	(oldest-first notmuch-search-oldest-first)
> > +	(maxitems notmuch-search-maxitems)
> >  	(target-thread (notmuch-search-find-thread-id))
> >  	(query notmuch-search-query-string)
> >  	(continuation notmuch-search-continuation))
> >      (notmuch-kill-this-buffer)
> > -    (notmuch-search query oldest-first target-thread target-line continuation)
> > +    (notmuch-search query oldest-first maxitems target-thread target-line continuation)
> >      (goto-char (point-min))))
> >  
> > +(defun notmuch-search-double-results (&rest ignore)
> > +  (if notmuch-search-maxitems
> > +      (set 'notmuch-search-maxitems (* 2 notmuch-search-maxitems)))
> > +  (notmuch-search-refresh-view))
> > +
> > +(defun notmuch-search-unlimited-results (&rest ignore)
> > +  (set 'notmuch-search-maxitems nil)
> > +  (notmuch-search-refresh-view))
> 
> I would recommend setq instead of set, except in the weird local
> places where `set' is clearly the prevailing style.

There's actually quite a bit of set around, but not enough to make it
the prevailing style. Will fix.

> > + 
> > +(defun notmuch-search-setup-buttons ()
> > +  (widget-insert "    ")
> > +  (widget-create 'push-button 
> > +		 :notify 'notmuch-search-double-results
> > +		 :help-echo "Double the number of results shown"
> > +		 "Show 2X results")
> > +  (widget-insert "    ")
> > +  (widget-create 'push-button
> > +		 :notify 'notmuch-search-unlimited-results
> > +		 :help-echo "Show all search results"
> > +		 "Show unlimited results")
> > +  (widget-setup))
> > +
> 
> If you're limiting from the bottom, would it make more sense for these
> widgets to be placed at the top of the buffer, since that's the
> direction they'll expand the results?

I'm always limiting from the oldest mails (regardless of the sort order)
but having the buttons at the top or bottom depending on where the
results are expanded does make sense.

BR,
Jani.

> >  (defcustom notmuch-poll-script ""
> >    "An external script to incorporate new mail into the notmuch database.
> >  
> > @@ -997,7 +1040,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)))
> >  
> >  (defun notmuch-search-filter-by-tag (tag)
> >    "Filter the current search results based on a single tag.
> > @@ -1006,7 +1049,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))
> >  
> >  ;;;###autoload
> >  (defun notmuch ()
> 
> -- 
> Austin Clements                                      MIT/'06/PhD/CSAIL
> amdragon@mit.edu                           http://web.mit.edu/amdragon
>        Somewhere in the dream we call reality you will find me,
>               searching for the reality we call dreams.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2011-11-06 20:57 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-31 21:18 [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages in search results Jani Nikula
2011-10-31 21:18 ` [RFC PATCH v2 1/3] lib: add function to get the number of threads matching a search Jani Nikula
2011-10-31 21:18 ` [RFC PATCH v2 2/3] cli: add options --first and --maxitems to notmuch search Jani Nikula
2011-10-31 21:18 ` [RFC PATCH v2 3/3] emacs: support limiting the number of results shown in search results Jani Nikula
2011-11-04 20:54   ` Austin Clements
2011-11-06 20:57     ` Jani Nikula
2011-10-31 21:44 ` [RFC PATCH v2 0/3] lib/cli/emacs: limit number of messages " Jameson Graef Rollins
2011-10-31 22:07   ` Jani Nikula
2011-10-31 22:19     ` Jameson Graef Rollins
2011-11-04 20:21     ` Austin Clements
2011-11-01 12:30 ` David Bremner
2011-11-02  9:27   ` Jani Nikula

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).