Re: [PATCH v4 1/6] cli: search: Refactor passing of command line options
authorMichal Sojka <sojkam1@fel.cvut.cz>
Thu, 30 Oct 2014 21:14:04 +0000 (22:14 +0100)
committerW. Trevor King <wking@tremily.us>
Fri, 7 Nov 2014 18:05:53 +0000 (10:05 -0800)
85/b3bbf95a0148abccf1869e4f3c417d5e809900 [new file with mode: 0644]

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