Re: [PATCH v4 1/6] cli: search: Refactor passing of command line options
authorMark Walters <markwalters1009@gmail.com>
Thu, 30 Oct 2014 07:57:13 +0000 (07:57 +0000)
committerW. Trevor King <wking@tremily.us>
Fri, 7 Nov 2014 18:05:52 +0000 (10:05 -0800)
7f/e49b5935ce5fcd72b58173e0b35a065fa6639e [new file with mode: 0644]

diff --git a/7f/e49b5935ce5fcd72b58173e0b35a065fa6639e b/7f/e49b5935ce5fcd72b58173e0b35a065fa6639e
new file mode 100644 (file)
index 0000000..ce8ac5f
--- /dev/null
@@ -0,0 +1,402 @@
+Return-Path: <m.walters@qmul.ac.uk>\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 4EDB7431FC2\r
+       for <notmuch@notmuchmail.org>; Thu, 30 Oct 2014 00:57:32 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -1.098\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5\r
+       tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001,\r
+       NML_ADSP_CUSTOM_MED=1.2, 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 yzYOKyUeujCO for <notmuch@notmuchmail.org>;\r
+       Thu, 30 Oct 2014 00:57:24 -0700 (PDT)\r
+Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6])\r
+       (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
+       (No client certificate requested)\r
+       by olra.theworths.org (Postfix) with ESMTPS id CD92D431FB6\r
+       for <notmuch@notmuchmail.org>; Thu, 30 Oct 2014 00:57:23 -0700 (PDT)\r
+Received: from smtp.qmul.ac.uk ([138.37.6.40])\r
+       by mail2.qmul.ac.uk with esmtp (Exim 4.71)\r
+       (envelope-from <m.walters@qmul.ac.uk>)\r
+       id 1Xjkbc-0002Co-7F; Thu, 30 Oct 2014 07:57:18 +0000\r
+Received: from 5751dfa2.skybroadband.com ([87.81.223.162] helo=localhost)\r
+       by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.71)\r
+       (envelope-from <m.walters@qmul.ac.uk>)\r
+       id 1Xjkbb-0006HX-Ao; Thu, 30 Oct 2014 07:57:15 +0000\r
+From: Mark Walters <markwalters1009@gmail.com>\r
+To: Michal Sojka <sojkam1@fel.cvut.cz>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH v4 1/6] cli: search: Refactor passing of command line\r
+       options\r
+In-Reply-To: <1414421455-3037-2-git-send-email-sojkam1@fel.cvut.cz>\r
+References: <1414421455-3037-1-git-send-email-sojkam1@fel.cvut.cz>\r
+       <1414421455-3037-2-git-send-email-sojkam1@fel.cvut.cz>\r
+User-Agent: Notmuch/0.18.1+86~gef5e66a (http://notmuchmail.org) Emacs/23.4.1\r
+       (x86_64-pc-linux-gnu)\r
+Date: Thu, 30 Oct 2014 07:57:13 +0000\r
+Message-ID: <87k33iuh0m.fsf@qmul.ac.uk>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+X-Sender-Host-Address: 87.81.223.162\r
+X-QM-Geographic: According to ripencc,\r
+       this message was delivered by a machine in Britain (UK) (GB).\r
+X-QM-SPAM-Info: Sender has good ham record.  :)\r
+X-QM-Body-MD5: 437f328dfd942f72761fd2e8207125bd (of first 20000 bytes)\r
+X-SpamAssassin-Score: -0.1\r
+X-SpamAssassin-SpamBar: /\r
+X-SpamAssassin-Report: The QM spam filters have analysed this message to\r
+       determine if it is\r
+       spam. We require at least 5.0 points to mark a message as spam.\r
+       This message scored -0.1 points.\r
+       Summary of the scoring: \r
+       * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail\r
+       provider *      (markwalters1009[at]gmail.com)\r
+       * -0.1 AWL AWL: From: address is in the auto white-list\r
+X-QM-Scan-Virus: ClamAV says the message is clean\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 07:57:32 -0000\r
+\r
+\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
+Otherwise this looks good to me.\r
+\r
+Best wishes\r
+\r
+Mark\r
+\r
+>      notmuch_messages_t *messages = NULL;\r
+>      notmuch_tags_t *tags;\r
+>      const char *tag;\r
+> +    sprinter_t *format = opt->format;\r
+> +    notmuch_query_t *query = opt->query;\r
+>  \r
+>      /* should the following only special case if no excluded terms\r
+>       * specified? */\r
+> @@ -333,16 +336,16 @@ int\r
+>  notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])\r
+>  {\r
+>      notmuch_database_t *notmuch;\r
+> -    notmuch_query_t *query;\r
+> +    search_options_t opt = {\r
+> +    .sort = NOTMUCH_SORT_NEWEST_FIRST,\r
+> +    .output = OUTPUT_SUMMARY,\r
+> +    .offset = 0,\r
+> +    .limit = -1, /* unlimited */\r
+> +    .dupe = -1,\r
+> +    };\r
+>      char *query_str;\r
+> -    notmuch_sort_t sort = NOTMUCH_SORT_NEWEST_FIRST;\r
+> -    sprinter_t *format = NULL;\r
+>      int opt_index, ret;\r
+> -    output_t output = OUTPUT_SUMMARY;\r
+> -    int offset = 0;\r
+> -    int limit = -1; /* unlimited */\r
+>      notmuch_exclude_t exclude = NOTMUCH_EXCLUDE_TRUE;\r
+> -    int dupe = -1;\r
+>      unsigned int i;\r
+>  \r
+>      enum {\r
+> @@ -353,7 +356,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])\r
+>      } format_sel = NOTMUCH_FORMAT_TEXT;\r
+>  \r
+>      notmuch_opt_desc_t options[] = {\r
+> -    { NOTMUCH_OPT_KEYWORD, &sort, "sort", 's',\r
+> +    { NOTMUCH_OPT_KEYWORD, &opt.sort, "sort", 's',\r
+>        (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },\r
+>                                { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },\r
+>                                { 0, 0 } } },\r
+> @@ -364,7 +367,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])\r
+>                                { "text0", NOTMUCH_FORMAT_TEXT0 },\r
+>                                { 0, 0 } } },\r
+>      { NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },\r
+> -    { NOTMUCH_OPT_KEYWORD, &output, "output", 'o',\r
+> +    { NOTMUCH_OPT_KEYWORD, &opt.output, "output", 'o',\r
+>        (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },\r
+>                                { "threads", OUTPUT_THREADS },\r
+>                                { "messages", OUTPUT_MESSAGES },\r
+> @@ -377,9 +380,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])\r
+>                                    { "flag", NOTMUCH_EXCLUDE_FLAG },\r
+>                                    { "all", NOTMUCH_EXCLUDE_ALL },\r
+>                                    { 0, 0 } } },\r
+> -    { NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 },\r
+> -    { NOTMUCH_OPT_INT, &limit, "limit", 'L', 0  },\r
+> -    { NOTMUCH_OPT_INT, &dupe, "duplicate", 'D', 0  },\r
+> +    { NOTMUCH_OPT_INT, &opt.offset, "offset", 'O', 0 },\r
+> +    { NOTMUCH_OPT_INT, &opt.limit, "limit", 'L', 0  },\r
+> +    { NOTMUCH_OPT_INT, &opt.dupe, "duplicate", 'D', 0  },\r
+>      { 0, 0, 0, 0, 0 }\r
+>      };\r
+>  \r
+> @@ -389,20 +392,20 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])\r
+>  \r
+>      switch (format_sel) {\r
+>      case NOTMUCH_FORMAT_TEXT:\r
+> -    format = sprinter_text_create (config, stdout);\r
+> +    opt.format = sprinter_text_create (config, stdout);\r
+>      break;\r
+>      case NOTMUCH_FORMAT_TEXT0:\r
+> -    if (output == OUTPUT_SUMMARY) {\r
+> +    if (opt.output == OUTPUT_SUMMARY) {\r
+>          fprintf (stderr, "Error: --format=text0 is not compatible with --output=summary.\n");\r
+>          return EXIT_FAILURE;\r
+>      }\r
+> -    format = sprinter_text0_create (config, stdout);\r
+> +    opt.format = sprinter_text0_create (config, stdout);\r
+>      break;\r
+>      case NOTMUCH_FORMAT_JSON:\r
+> -    format = sprinter_json_create (config, stdout);\r
+> +    opt.format = sprinter_json_create (config, stdout);\r
+>      break;\r
+>      case NOTMUCH_FORMAT_SEXP:\r
+> -    format = sprinter_sexp_create (config, stdout);\r
+> +    opt.format = sprinter_sexp_create (config, stdout);\r
+>      break;\r
+>      default:\r
+>      /* this should never happen */\r
+> @@ -425,15 +428,15 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])\r
+>      return EXIT_FAILURE;\r
+>      }\r
+>  \r
+> -    query = notmuch_query_create (notmuch, query_str);\r
+> -    if (query == NULL) {\r
+> +    opt.query = notmuch_query_create (notmuch, query_str);\r
+> +    if (opt.query == NULL) {\r
+>      fprintf (stderr, "Out of memory\n");\r
+>      return EXIT_FAILURE;\r
+>      }\r
+>  \r
+> -    notmuch_query_set_sort (query, sort);\r
+> +    notmuch_query_set_sort (opt.query, opt.sort);\r
+>  \r
+> -    if (exclude == NOTMUCH_EXCLUDE_FLAG && output != OUTPUT_SUMMARY) {\r
+> +    if (exclude == NOTMUCH_EXCLUDE_FLAG && opt.output != OUTPUT_SUMMARY) {\r
+>      /* If we are not doing summary output there is nowhere to\r
+>       * print the excluded flag so fall back on including the\r
+>       * excluded messages. */\r
+> @@ -448,29 +451,29 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])\r
+>      search_exclude_tags = notmuch_config_get_search_exclude_tags\r
+>          (config, &search_exclude_tags_length);\r
+>      for (i = 0; i < search_exclude_tags_length; i++)\r
+> -        notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);\r
+> -    notmuch_query_set_omit_excluded (query, exclude);\r
+> +        notmuch_query_add_tag_exclude (opt.query, search_exclude_tags[i]);\r
+> +    notmuch_query_set_omit_excluded (opt.query, exclude);\r
+>      }\r
+>  \r
+> -    switch (output) {\r
+> +    switch (opt.output) {\r
+>      default:\r
+>      case OUTPUT_SUMMARY:\r
+>      case OUTPUT_THREADS:\r
+> -    ret = do_search_threads (format, query, sort, output, offset, limit);\r
+> +    ret = do_search_threads (&opt);\r
+>      break;\r
+>      case OUTPUT_MESSAGES:\r
+>      case OUTPUT_FILES:\r
+> -    ret = do_search_messages (format, query, output, offset, limit, dupe);\r
+> +    ret = do_search_messages (&opt);\r
+>      break;\r
+>      case OUTPUT_TAGS:\r
+> -    ret = do_search_tags (notmuch, format, query);\r
+> +    ret = do_search_tags (notmuch, &opt);\r
+>      break;\r
+>      }\r
+>  \r
+> -    notmuch_query_destroy (query);\r
+> +    notmuch_query_destroy (opt.query);\r
+>      notmuch_database_destroy (notmuch);\r
+>  \r
+> -    talloc_free (format);\r
+> +    talloc_free (opt.format);\r
+>  \r
+>      return ret ? EXIT_FAILURE : EXIT_SUCCESS;\r
+>  }\r
+> -- \r
+> 2.1.1\r
+>\r
+> _______________________________________________\r
+> notmuch mailing list\r
+> notmuch@notmuchmail.org\r
+> http://notmuchmail.org/mailman/listinfo/notmuch\r