From b48783a6e4341482c48bb0add917aed661a69315 Mon Sep 17 00:00:00 2001 From: Michal Sojka Date: Thu, 30 Oct 2014 22:14:04 +0100 Subject: [PATCH] Re: [PATCH v4 1/6] cli: search: Refactor passing of command line options --- 85/b3bbf95a0148abccf1869e4f3c417d5e809900 | 237 ++++++++++++++++++++++ 1 file changed, 237 insertions(+) create mode 100644 85/b3bbf95a0148abccf1869e4f3c417d5e809900 diff --git a/85/b3bbf95a0148abccf1869e4f3c417d5e809900 b/85/b3bbf95a0148abccf1869e4f3c417d5e809900 new file mode 100644 index 000000000..fb88c9407 --- /dev/null +++ b/85/b3bbf95a0148abccf1869e4f3c417d5e809900 @@ -0,0 +1,237 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 0A601431FD0 + for ; Thu, 30 Oct 2014 14:14:26 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -2.3 +X-Spam-Level: +X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_MED=-2.3] 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 efgAb6eM-7lM for ; + Thu, 30 Oct 2014 14:14:18 -0700 (PDT) +Received: from max.feld.cvut.cz (max.feld.cvut.cz [147.32.192.36]) + by olra.theworths.org (Postfix) with ESMTP id 6D7EA431FCB + for ; Thu, 30 Oct 2014 14:14:18 -0700 (PDT) +Received: from localhost (unknown [192.168.200.7]) + by max.feld.cvut.cz (Postfix) with ESMTP id 7B66619F373C; + Thu, 30 Oct 2014 22:14:15 +0100 (CET) +X-Virus-Scanned: IMAP STYX AMAVIS +Received: from max.feld.cvut.cz ([192.168.200.1]) + by localhost (styx.feld.cvut.cz [192.168.200.7]) (amavisd-new, + port 10044) + with ESMTP id k_YH2tZCUAxV; Thu, 30 Oct 2014 22:14:11 +0100 (CET) +Received: from imap.feld.cvut.cz (imap.feld.cvut.cz [147.32.192.34]) + by max.feld.cvut.cz (Postfix) with ESMTP id 1DCF719F373D; + Thu, 30 Oct 2014 22:14:11 +0100 (CET) +Received: from wsh by steelpick.2x.cz with local (Exim 4.84) + (envelope-from ) + id 1Xjx2i-0005ca-9c; Thu, 30 Oct 2014 22:14:04 +0100 +From: Michal Sojka +To: Mark Walters , notmuch@notmuchmail.org +Subject: Re: [PATCH v4 1/6] cli: search: Refactor passing of command line + options +In-Reply-To: <87k33iuh0m.fsf@qmul.ac.uk> +References: <1414421455-3037-1-git-send-email-sojkam1@fel.cvut.cz> + <1414421455-3037-2-git-send-email-sojkam1@fel.cvut.cz> + <87k33iuh0m.fsf@qmul.ac.uk> +User-Agent: Notmuch/0.18.2+157~ga00d359 (http://notmuchmail.org) Emacs/24.3.1 + (x86_64-pc-linux-gnu) +Date: Thu, 30 Oct 2014 22:14:04 +0100 +Message-ID: <878ujxclb7.fsf@steelpick.2x.cz> +MIME-Version: 1.0 +Content-Type: text/plain +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: Thu, 30 Oct 2014 21:14:26 -0000 + +Hi Mark, + +On Thu, Oct 30 2014, Mark Walters wrote: +> Hi +> +> On Mon, 27 Oct 2014, Michal Sojka wrote: +>> Many functions that implement the search command need to access command +>> line options. Instead of passing each option in a separate variable, put +>> them in a structure and pass only this structure. +>> +>> This will become handy in the following patches. +>> --- +>> notmuch-search.c | 125 ++++++++++++++++++++++++++++--------------------------- +>> 1 file changed, 64 insertions(+), 61 deletions(-) +>> +>> diff --git a/notmuch-search.c b/notmuch-search.c +>> index bc9be45..0c3e972 100644 +>> --- a/notmuch-search.c +>> +++ b/notmuch-search.c +>> @@ -30,6 +30,16 @@ typedef enum { +>> OUTPUT_TAGS +>> } output_t; +>> +>> +typedef struct { +>> + sprinter_t *format; +>> + notmuch_query_t *query; +>> + notmuch_sort_t sort; +>> + output_t output; +>> + int offset; +>> + int limit; +>> + int dupe; +>> +} search_options_t; +>> + +>> /* Return two stable query strings that identify exactly the matched +>> * and unmatched messages currently in thread. If there are no +>> * matched or unmatched messages, the returned buffers will be +>> @@ -70,43 +80,39 @@ get_thread_query (notmuch_thread_t *thread, +>> } +>> +>> static int +>> -do_search_threads (sprinter_t *format, +>> - notmuch_query_t *query, +>> - notmuch_sort_t sort, +>> - output_t output, +>> - int offset, +>> - int limit) +>> +do_search_threads (search_options_t *opt) +>> { +>> notmuch_thread_t *thread; +>> notmuch_threads_t *threads; +>> notmuch_tags_t *tags; +>> + sprinter_t *format = opt->format; +>> time_t date; +>> int i; +>> +>> - if (offset < 0) { +>> - offset += notmuch_query_count_threads (query); +>> - if (offset < 0) +>> - offset = 0; +>> + if (opt->offset < 0) { +>> + opt->offset += notmuch_query_count_threads (opt->query); +>> + if (opt->offset < 0) +>> + opt->offset = 0; +>> } +>> +>> - threads = notmuch_query_search_threads (query); +>> + threads = notmuch_query_search_threads (opt->query); +>> if (threads == NULL) +>> return 1; +>> +>> format->begin_list (format); +>> +>> for (i = 0; +>> - notmuch_threads_valid (threads) && (limit < 0 || i < offset + limit); +>> + notmuch_threads_valid (threads) && (opt->limit < 0 || i < opt->offset + opt->limit); +>> notmuch_threads_move_to_next (threads), i++) +>> { +>> thread = notmuch_threads_get (threads); +>> +>> - if (i < offset) { +>> + if (i < opt->offset) { +>> notmuch_thread_destroy (thread); +>> continue; +>> } +>> +>> - if (output == OUTPUT_THREADS) { +>> + if (opt->output == OUTPUT_THREADS) { +>> format->set_prefix (format, "thread"); +>> format->string (format, +>> notmuch_thread_get_thread_id (thread)); +>> @@ -123,7 +129,7 @@ do_search_threads (sprinter_t *format, +>> +>> format->begin_map (format); +>> +>> - if (sort == NOTMUCH_SORT_OLDEST_FIRST) +>> + if (opt->sort == NOTMUCH_SORT_OLDEST_FIRST) +>> date = notmuch_thread_get_oldest_date (thread); +>> else +>> date = notmuch_thread_get_newest_date (thread); +>> @@ -215,40 +221,36 @@ do_search_threads (sprinter_t *format, +>> } +>> +>> static int +>> -do_search_messages (sprinter_t *format, +>> - notmuch_query_t *query, +>> - output_t output, +>> - int offset, +>> - int limit, +>> - int dupe) +>> +do_search_messages (search_options_t *opt) +>> { +>> notmuch_message_t *message; +>> notmuch_messages_t *messages; +>> notmuch_filenames_t *filenames; +>> + sprinter_t *format = opt->format; +>> int i; +>> +>> - if (offset < 0) { +>> - offset += notmuch_query_count_messages (query); +>> - if (offset < 0) +>> - offset = 0; +>> + if (opt->offset < 0) { +>> + opt->offset += notmuch_query_count_messages (opt->query); +>> + if (opt->offset < 0) +>> + opt->offset = 0; +>> } +>> +>> - messages = notmuch_query_search_messages (query); +>> + messages = notmuch_query_search_messages (opt->query); +>> if (messages == NULL) +>> return 1; +>> +>> format->begin_list (format); +>> +>> for (i = 0; +>> - notmuch_messages_valid (messages) && (limit < 0 || i < offset + limit); +>> + notmuch_messages_valid (messages) && (opt->limit < 0 || i < opt->offset + opt->limit); +>> notmuch_messages_move_to_next (messages), i++) +>> { +>> - if (i < offset) +>> + if (i < opt->offset) +>> continue; +>> +>> message = notmuch_messages_get (messages); +>> +>> - if (output == OUTPUT_FILES) { +>> + if (opt->output == OUTPUT_FILES) { +>> int j; +>> filenames = notmuch_message_get_filenames (message); +>> +>> @@ -256,7 +258,7 @@ do_search_messages (sprinter_t *format, +>> notmuch_filenames_valid (filenames); +>> notmuch_filenames_move_to_next (filenames), j++) +>> { +>> - if (dupe < 0 || dupe == j) { +>> + if (opt->dupe < 0 || opt->dupe == j) { +>> format->string (format, notmuch_filenames_get (filenames)); +>> format->separator (format); +>> } +>> @@ -283,12 +285,13 @@ do_search_messages (sprinter_t *format, +>> +>> static int +>> do_search_tags (notmuch_database_t *notmuch, +>> - sprinter_t *format, +>> - notmuch_query_t *query) +>> + const search_options_t *opt) +>> { +> +> What about leaving out the *notmuch argument and getting that from the +> query (query->notmuch I think)? + +I was also thinking about this, but notmuch_query_t is an opaque +structure which means that we would need a getter function in the +library. I think it is simpler to have it this way. + +-Michal -- 2.26.2