79/c2913881feb219ec352ff19c56a0dec4b1f460

   1 Return-Path: <tomi.ollila@iki.fi>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 2FAE0431FB6\r
   6         for <notmuch@notmuchmail.org>; Fri, 21 Jun 2013 04:00:15 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: 0\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
  12         autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id ANE+3NO6YFQh for <notmuch@notmuchmail.org>;\r
  16         Fri, 21 Jun 2013 04:00:08 -0700 (PDT)\r
  17 Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])\r
  18         by olra.theworths.org (Postfix) with ESMTP id 845CE431FAE\r
  19         for <notmuch@notmuchmail.org>; Fri, 21 Jun 2013 04:00:08 -0700 (PDT)\r
  20 Received: from guru.guru-group.fi (localhost [IPv6:::1])\r
  21         by guru.guru-group.fi (Postfix) with ESMTP id 4FB66100033;\r
  22         Fri, 21 Jun 2013 13:59:57 +0300 (EEST)\r
  23 From: Tomi Ollila <tomi.ollila@iki.fi>\r
  24 To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org\r
  25 Subject: Re: [PATCH v3 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t\r
  26 In-Reply-To: <87fvwdx19x.fsf@qmul.ac.uk>\r
  27 References: <1368301809-12532-1-git-send-email-markwalters1009@gmail.com>\r
  28         <1368301809-12532-8-git-send-email-markwalters1009@gmail.com>\r
  29         <87bo8edif8.fsf@qmul.ac.uk> <87fvwdx19x.fsf@qmul.ac.uk>\r
  30 User-Agent: Notmuch/0.15.2+181~gfb53171 (http://notmuchmail.org) Emacs/24.3.1\r
  31         (x86_64-unknown-linux-gnu)\r
  32 X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL\r
  33         $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F\r
  34         !)g;OY^,BjTbr)Np:%c_o'jj,Z\r
  35 Date: Fri, 21 Jun 2013 13:59:57 +0300\r
  36 Message-ID: <m2txkrk9ua.fsf@guru.guru-group.fi>\r
  37 MIME-Version: 1.0\r
  38 Content-Type: text/plain\r
  39 X-BeenThere: notmuch@notmuchmail.org\r
  40 X-Mailman-Version: 2.1.13\r
  41 Precedence: list\r
  42 List-Id: "Use and development of the notmuch mail system."\r
  43         <notmuch.notmuchmail.org>\r
  44 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  45         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  46 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  47 List-Post: <mailto:notmuch@notmuchmail.org>\r
  48 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  49 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  50         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  51 X-List-Received-Date: Fri, 21 Jun 2013 11:00:15 -0000\r
  52 \r
  53 On Thu, Jun 20 2013, Mark Walters <markwalters1009@gmail.com> wrote:\r
  54 \r
  55 > I think we should decide before 0.16 whether to go with this patch and\r
  56 > patch 8/8 or for Peter's suggestion at\r
  57 > id:1368454815-1854-1-git-send-email-novalazy@gmail.com\r
  58 >\r
  59 > Now we have an enum for the NOTMUCH_EXCLUDE possibilities (rather than a\r
  60 > bool) I think it makes sense to implement NOTMUCH_EXCLUDE_FALSE properly\r
  61 > but I am happy with either choice.\r
  62 \r
  63 So, the choice here is to choose between\r
  64 \r
  65 id:"1368454815-1854-1-git-send-email-novalazy@gmail.com"\r
  66 and\r
  67 id:"87bo8edif8.fsf@qmul.ac.uk" \r
  68 \r
  69 (might be hard to catch from this thread -- was easier to take from\r
  70 http://nmbug.tethera.net/status/ )\r
  71 \r
  72 Both apply cleanly to current master ( d0bd88f )\r
  73 \r
  74 While Peter's version surely looks simpler (and may work, didn't test atm)\r
  75 the comments Mark presents makes sense (especially the "subtlety" thing ;)\r
  76 \r
  77 IMHO Mark's version makes future development "safer" and therefore my\r
  78 vote (or million of those) goes to id:"87bo8edif8.fsf@qmul.ac.uk"\r
  79 \r
  80 > Best wishes\r
  81 >\r
  82 > Mark\r
  83 \r
  84 Tomi\r
  85 \r
  86 \r
  87 >\r
  88 >\r
  89 >\r
  90 > On Mon, 13 May 2013, Mark Walters <markwalters1009@gmail.com> wrote:\r
  91 >> Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can\r
  92 >> cover all four values of search --exclude in the cli.\r
  93 >>\r
  94 >> Previously the way to avoid any message being marked excluded was to\r
  95 >> pass in an empty list of excluded tags: since we now have an explicit\r
  96 >> option we might as well honour it.\r
  97 >>\r
  98 >> The enum is in a slightly strange order as the existing FALSE/TRUE\r
  99 >> options correspond to the new\r
 100 >> NOTMUCH_EXCLUDE_FLAG/NOTMUCH_EXCLUDE_TRUE options so this means we do\r
 101 >> not need to bump the version number.\r
 102 >>\r
 103 >> Indeed, an example of this is that the cli count and show still use\r
 104 >> FALSE/TRUE and still work.\r
 105 >> ---\r
 106 >>\r
 107 >> This is a new version of this single patch. In Peter's version\r
 108 >> NOTMUCH_EXCLUDE_FLAG and NOTMUCH_EXCLUDE_FALSE were treated as synonyms:\r
 109 >> this makes them behave in the obvious way and documents them.\r
 110 >>\r
 111 >> I think the only subtlety is the enum order mentioned in the commit\r
 112 >> message. Additionally it would be good to update cli count and show and,\r
 113 >> at for the latter, it would be good to add an option exclude=all too (so\r
 114 >> excluded messages are completely omitted). Those should both be simple\r
 115 >> but we need to decide whether to allow all four options\r
 116 >> (false/flag/true/all) always or not. Always allowing all 4 is nicely\r
 117 >> consistent but sometimes redundant. Additionally we would need some\r
 118 >> tests.\r
 119 >>\r
 120 >> I think this patch should go in with the main series as I think it is\r
 121 >> worth reserving the NOTMUCH_EXCLUDE_FALSE #define/value so that we don't\r
 122 >> break code in future if we do wish to add it.\r
 123 >>\r
 124 >> Best wishes\r
 125 >>\r
 126 >> Mark\r
 127 >>\r
 128 >>\r
 129 >>  \r
 130 >>  lib/notmuch.h    |   18 ++++++++++++++++--\r
 131 >>  lib/query.cc     |    8 +++++---\r
 132 >>  lib/thread.cc    |   28 +++++++++++++++-------------\r
 133 >>  notmuch-search.c |    2 +-\r
 134 >>  4 files changed, 37 insertions(+), 19 deletions(-)\r
 135 >>\r
 136 >> diff --git a/lib/notmuch.h b/lib/notmuch.h\r
 137 >> index 27b43ff..73c85a4 100644\r
 138 >> --- a/lib/notmuch.h\r
 139 >> +++ b/lib/notmuch.h\r
 140 >> @@ -500,10 +500,15 @@ typedef enum {\r
 141 >>  const char *\r
 142 >>  notmuch_query_get_query_string (notmuch_query_t *query);\r
 143 >>  \r
 144 >> -/* Exclude values for notmuch_query_set_omit_excluded */\r
 145 >> +/* Exclude values for notmuch_query_set_omit_excluded. The strange\r
 146 >> + * order is to maintain backward compatibility: the old FALSE/TRUE\r
 147 >> + * options correspond to the new\r
 148 >> + * NOTMUCH_EXCLUDE_FLAG/NOTMUCH_EXCLUDE_TRUE options.\r
 149 >> + */\r
 150 >>  typedef enum {\r
 151 >> -    NOTMUCH_EXCLUDE_FALSE,\r
 152 >> +    NOTMUCH_EXCLUDE_FLAG,\r
 153 >>      NOTMUCH_EXCLUDE_TRUE,\r
 154 >> +    NOTMUCH_EXCLUDE_FALSE,\r
 155 >>      NOTMUCH_EXCLUDE_ALL\r
 156 >>  } notmuch_exclude_t;\r
 157 >>  \r
 158 >> @@ -517,6 +522,15 @@ typedef enum {\r
 159 >>   * match in at least one non-excluded message.  Otherwise, if set to ALL,\r
 160 >>   * notmuch_query_search_threads will omit excluded messages from all threads.\r
 161 >>   *\r
 162 >> + * If set to FALSE or FLAG then both notmuch_query_search_messages and\r
 163 >> + * notmuch_query_search_threads will return all matching\r
 164 >> + * messages/threads regardless of exclude status. If set to FLAG then\r
 165 >> + * the exclude flag will be set for any excluded message that is\r
 166 >> + * returned by notmuch_query_search_messages, and the thread counts\r
 167 >> + * for threads returned by notmuch_query_search_threads will be the\r
 168 >> + * number of non-excluded messages/matches. Otherwise, if set to\r
 169 >> + * FALSE, then the exclude status is completely ignored.\r
 170 >> + *\r
 171 >>   * The performance difference when calling\r
 172 >>   * notmuch_query_search_messages should be relatively small (and both\r
 173 >>   * should be very fast).  However, in some cases,\r
 174 >> diff --git a/lib/query.cc b/lib/query.cc\r
 175 >> index 1cc768f..69668a4 100644\r
 176 >> --- a/lib/query.cc\r
 177 >> +++ b/lib/query.cc\r
 178 >> @@ -218,13 +218,15 @@ notmuch_query_search_messages (notmuch_query_t *query)\r
 179 >>      }\r
 180 >>      messages->base.excluded_doc_ids = NULL;\r
 181 >>  \r
 182 >> -    if (query->exclude_terms) {\r
 183 >> +    if ((query->omit_excluded != NOTMUCH_EXCLUDE_FALSE) && (query->exclude_terms)) {\r
 184 >>          exclude_query = _notmuch_exclude_tags (query, final_query);\r
 185 >>  \r
 186 >> -        if (query->omit_excluded != NOTMUCH_EXCLUDE_FALSE)\r
 187 >> +        if (query->omit_excluded == NOTMUCH_EXCLUDE_TRUE ||\r
 188 >> +            query->omit_excluded == NOTMUCH_EXCLUDE_ALL)\r
 189 >> +        {\r
 190 >>              final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,\r
 191 >>                                           final_query, exclude_query);\r
 192 >> -        else {\r
 193 >> +        } else { /* NOTMUCH_EXCLUDE_FLAG */\r
 194 >>              exclude_query = Xapian::Query (Xapian::Query::OP_AND,\r
 195 >>                                         exclude_query, final_query);\r
 196 >>  \r
 197 >> diff --git a/lib/thread.cc b/lib/thread.cc\r
 198 >> index bc07877..4dcf705 100644\r
 199 >> --- a/lib/thread.cc\r
 200 >> +++ b/lib/thread.cc\r
 201 >> @@ -238,20 +238,22 @@ _thread_add_message (notmuch_thread_t *thread,\r
 202 >>      char *clean_author;\r
 203 >>      notmuch_bool_t message_excluded = FALSE;\r
 204 >>  \r
 205 >> -    for (tags = notmuch_message_get_tags (message);\r
 206 >> -     notmuch_tags_valid (tags);\r
 207 >> -     notmuch_tags_move_to_next (tags))\r
 208 >> -    {\r
 209 >> -    tag = notmuch_tags_get (tags);\r
 210 >> -    /* Is message excluded? */\r
 211 >> -    for (notmuch_string_node_t *term = exclude_terms->head;\r
 212 >> -         term != NULL;\r
 213 >> -         term = term->next)\r
 214 >> +    if (omit_exclude != NOTMUCH_EXCLUDE_FALSE) {\r
 215 >> +    for (tags = notmuch_message_get_tags (message);\r
 216 >> +         notmuch_tags_valid (tags);\r
 217 >> +         notmuch_tags_move_to_next (tags))\r
 218 >>      {\r
 219 >> -        /* We ignore initial 'K'. */\r
 220 >> -        if (strcmp(tag, (term->string + 1)) == 0) {\r
 221 >> -            message_excluded = TRUE;\r
 222 >> -            break;\r
 223 >> +        tag = notmuch_tags_get (tags);\r
 224 >> +        /* Is message excluded? */\r
 225 >> +        for (notmuch_string_node_t *term = exclude_terms->head;\r
 226 >> +             term != NULL;\r
 227 >> +             term = term->next)\r
 228 >> +        {\r
 229 >> +            /* We ignore initial 'K'. */\r
 230 >> +            if (strcmp(tag, (term->string + 1)) == 0) {\r
 231 >> +                message_excluded = TRUE;\r
 232 >> +                break;\r
 233 >> +            }\r
 234 >>          }\r
 235 >>      }\r
 236 >>      }\r
 237 >> diff --git a/notmuch-search.c b/notmuch-search.c\r
 238 >> index 4323201..a20791a 100644\r
 239 >> --- a/notmuch-search.c\r
 240 >> +++ b/notmuch-search.c\r
 241 >> @@ -411,7 +411,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])\r
 242 >>      for (i = 0; i < search_exclude_tags_length; i++)\r
 243 >>          notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);\r
 244 >>      if (exclude == EXCLUDE_FLAG)\r
 245 >> -        notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FALSE);\r
 246 >> +        notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FLAG);\r
 247 >>      if (exclude == EXCLUDE_ALL)\r
 248 >>          notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_ALL);\r
 249 >>      }\r
 250 >> -- \r
 251 >> 1.7.9.1\r
 252 > _______________________________________________\r
 253 > notmuch mailing list\r
 254 > notmuch@notmuchmail.org\r
 255 > http://notmuchmail.org/mailman/listinfo/notmuch\r