41/cb0adafb6d355dd27849bdd99a791637f044af

   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 49148431FAF\r
   6         for <notmuch@notmuchmail.org>; Mon, 13 Oct 2014 12:01:24 -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 1xQpcXesZ2EW for <notmuch@notmuchmail.org>;\r
  16         Mon, 13 Oct 2014 12:01:20 -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 DB1EC431FAE\r
  19         for <notmuch@notmuchmail.org>; Mon, 13 Oct 2014 12:01:19 -0700 (PDT)\r
  20 Received: from guru.guru-group.fi (localhost [IPv6:::1])\r
  21         by guru.guru-group.fi (Postfix) with ESMTP id 148DB100090;\r
  22         Mon, 13 Oct 2014 22:00:59 +0300 (EEST)\r
  23 From: Tomi Ollila <tomi.ollila@iki.fi>\r
  24 To: Michal Sojka <sojkam1@fel.cvut.cz>, notmuch@notmuchmail.org\r
  25 Subject: Re: [PATCH v3 3/4] cli: Extend the search command for\r
  26         --output={sender, recipients}\r
  27 In-Reply-To: <1413150093-8383-4-git-send-email-sojkam1@fel.cvut.cz>\r
  28 References: <87zjd51phx.fsf@steelpick.2x.cz>\r
  29         <1413150093-8383-1-git-send-email-sojkam1@fel.cvut.cz>\r
  30         <1413150093-8383-4-git-send-email-sojkam1@fel.cvut.cz>\r
  31 User-Agent: Notmuch/0.18.1+130~ga61922f (http://notmuchmail.org) Emacs/24.3.1\r
  32         (x86_64-unknown-linux-gnu)\r
  33 X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL\r
  34         $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F\r
  35         !)g;OY^,BjTbr)Np:%c_o'jj,Z\r
  36 Date: Mon, 13 Oct 2014 22:00:58 +0300\r
  37 Message-ID: <m238arrdzp.fsf@guru.guru-group.fi>\r
  38 MIME-Version: 1.0\r
  39 Content-Type: text/plain\r
  40 X-BeenThere: notmuch@notmuchmail.org\r
  41 X-Mailman-Version: 2.1.13\r
  42 Precedence: list\r
  43 List-Id: "Use and development of the notmuch mail system."\r
  44         <notmuch.notmuchmail.org>\r
  45 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  46         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  47 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  48 List-Post: <mailto:notmuch@notmuchmail.org>\r
  49 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  50 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  51         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  52 X-List-Received-Date: Mon, 13 Oct 2014 19:01:24 -0000\r
  53 \r
  54 On Mon, Oct 13 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:\r
  55 \r
  56 > The new outputs allow printing senders, recipients or both of matching\r
  57 > messages. The --output option is converted from "keyword" argument to\r
  58 > "flags" argument, which means that the user can use --output=sender and\r
  59 > --output=recipients simultaneously, to print both. Other combinations\r
  60 > produce an error.\r
  61 >\r
  62 > This code based on a patch from Jani Nikula.\r
  63 > ---\r
  64 >  completion/notmuch-completion.bash |   2 +-\r
  65 >  completion/notmuch-completion.zsh  |   3 +-\r
  66 >  doc/man1/notmuch-search.rst        |  22 +++++++-\r
  67 >  notmuch-search.c                   | 110 ++++++++++++++++++++++++++++++++++---\r
  68 >  test/T090-search-output.sh         |  64 +++++++++++++++++++++\r
  69 >  5 files changed, 189 insertions(+), 12 deletions(-)\r
  70 >\r
  71 > diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash\r
  72 > index 0571dc9..cfbd389 100644\r
  73 > --- a/completion/notmuch-completion.bash\r
  74 > +++ b/completion/notmuch-completion.bash\r
  75 > @@ -294,7 +294,7 @@ _notmuch_search()\r
  76 >           return\r
  77 >           ;;\r
  78 >       --output)\r
  79 > -         COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )\r
  80 > +         COMPREPLY=( $( compgen -W "summary threads messages files tags sender recipients" -- "${cur}" ) )\r
  81 >           return\r
  82 >           ;;\r
  83 >       --sort)\r
  84 > diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh\r
  85 > index 67a9aba..3e52a00 100644\r
  86 > --- a/completion/notmuch-completion.zsh\r
  87 > +++ b/completion/notmuch-completion.zsh\r
  88 > @@ -52,7 +52,8 @@ _notmuch_search()\r
  89 >    _arguments -s : \\r
  90 >      '--max-threads=[display only the first x threads from the search results]:number of threads to show: ' \\r
  91 >      '--first=[omit the first x threads from the search results]:number of threads to omit: ' \\r
  92 > -    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))'\r
  93 > +    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \\r
  94 > +    '--output=[select what to output]:output:((summary threads messages files tags sender recipients))'\r
  95 >  }\r
  96 >  \r
  97 >  _notmuch()\r
  98 > diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst\r
  99 > index 90160f2..c9d38b1 100644\r
 100 > --- a/doc/man1/notmuch-search.rst\r
 101 > +++ b/doc/man1/notmuch-search.rst\r
 102 > @@ -35,7 +35,7 @@ Supported options for **search** include\r
 103 >          intended for programs that invoke **notmuch(1)** internally. If\r
 104 >          omitted, the latest supported version will be used.\r
 105 >  \r
 106 > -    ``--output=(summary|threads|messages|files|tags)``\r
 107 > +    ``--output=(summary|threads|messages|files|tags|sender|recipients)``\r
 108 >  \r
 109 >          **summary**\r
 110 >              Output a summary of each thread with any message matching\r
 111 > @@ -78,6 +78,26 @@ Supported options for **search** include\r
 112 >              by null characters (--format=text0), as a JSON array\r
 113 >              (--format=json), or as an S-Expression list (--format=sexp).\r
 114 >  \r
 115 > +     **sender**\r
 116 > +            Output all addresses from the *From* header that appear on\r
 117 > +            any message matching the search terms, either one per line\r
 118 > +            (--format=text), separated by null characters\r
 119 > +            (--format=text0), as a JSON array (--format=json), or as\r
 120 > +            an S-Expression list (--format=sexp).\r
 121 > +\r
 122 > +         Note: Searching for **sender** should be much faster than\r
 123 > +         searching for **recipients**, because sender addresses are\r
 124 > +         cached directly in the database whereas other addresses\r
 125 > +         need to be fetched from message files.\r
 126 > +\r
 127 > +     **recipients**\r
 128 > +            Like **sender** but for addresses from *To*, *Cc* and\r
 129 > +         *Bcc* headers.\r
 130 > +\r
 131 > +     This option can be given multiple times to combine different\r
 132 > +     outputs. Curently, this is only supported for **sender** and\r
 133 > +     **recipients** outputs.\r
 134 > +\r
 135 >      ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)\r
 136 >          This option can be used to present results in either\r
 137 >          chronological order (**oldest-first**) or reverse chronological\r
 138 > diff --git a/notmuch-search.c b/notmuch-search.c\r
 139 > index 5ac2a26..74588f8 100644\r
 140 > --- a/notmuch-search.c\r
 141 > +++ b/notmuch-search.c\r
 142 > @@ -23,11 +23,14 @@\r
 143 >  #include "string-util.h"\r
 144 >  \r
 145 >  typedef enum {\r
 146 > -    OUTPUT_SUMMARY,\r
 147 > -    OUTPUT_THREADS,\r
 148 > -    OUTPUT_MESSAGES,\r
 149 > -    OUTPUT_FILES,\r
 150 > -    OUTPUT_TAGS\r
 151 > +    OUTPUT_SUMMARY   = 1 << 0,\r
 152 > +    OUTPUT_THREADS   = 1 << 1,\r
 153 > +    OUTPUT_MESSAGES  = 1 << 2,\r
 154 > +    OUTPUT_FILES     = 1 << 3,\r
 155 > +    OUTPUT_TAGS              = 1 << 4,\r
 156 > +    OUTPUT_SENDER    = 1 << 5,\r
 157 > +    OUTPUT_RECIPIENTS        = 1 << 6,\r
 158 > +    OUTPUT_ADDRESSES = OUTPUT_SENDER | OUTPUT_RECIPIENTS,\r
 159 \r
 160 leftover, like mentioned below (this comment added just before sending)\r
 161 \r
 162 >  } output_t;\r
 163 >  \r
 164 >  typedef struct {\r
 165 > @@ -220,6 +223,67 @@ do_search_threads (search_options_t *o)\r
 166 >      return 0;\r
 167 >  }\r
 168 >  \r
 169 > +static void\r
 170 > +print_address_list (const search_options_t *o, InternetAddressList *list)\r
 171 > +{\r
 172 > +    InternetAddress *address;\r
 173 > +    int i;\r
 174 > +\r
 175 > +    for (i = 0; i < internet_address_list_length (list); i++) {\r
 176 > +     address = internet_address_list_get_address (list, i);\r
 177 > +     if (INTERNET_ADDRESS_IS_GROUP (address)) {\r
 178 > +         InternetAddressGroup *group;\r
 179 > +         InternetAddressList *group_list;\r
 180 > +\r
 181 > +         group = INTERNET_ADDRESS_GROUP (address);\r
 182 > +         group_list = internet_address_group_get_members (group);\r
 183 > +         if (group_list == NULL)\r
 184 > +             continue;\r
 185 > +\r
 186 > +         print_address_list (o, group_list);\r
 187 > +     } else {\r
 188 > +         InternetAddressMailbox *mailbox;\r
 189 > +         const char *name;\r
 190 > +         const char *addr;\r
 191 > +         char *full_address;\r
 192 > +\r
 193 > +         mailbox = INTERNET_ADDRESS_MAILBOX (address);\r
 194 > +\r
 195 > +         name = internet_address_get_name (address);\r
 196 > +         addr = internet_address_mailbox_get_addr (mailbox);\r
 197 > +\r
 198 > +         if (name && *name)\r
 199 > +             full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);\r
 200 > +         else\r
 201 > +             full_address = talloc_strdup (o->format, addr);\r
 202 > +\r
 203 > +         if (!full_address) {\r
 204 \r
 205 Apart from minor style issue like space after ! and the leftover ADDRESSES\r
 206 parts (w/o that I would not have commented about !<SPC>) the first 3\r
 207 patches look pretty good to me. I have not tested those yet.\r
 208 \r
 209 But we keep to have some disagreement w/ unique/duplicate/filter-by\r
 210 handling ;/\r
 211 \r
 212 I (currently) rest the case of first/last/most common handling to just\r
 213 how the --sort=(newest-first|oldest-first) affects the order...\r
 214 \r
 215 Let's consider the following list of output if no /deduplication/ is done:\r
 216 \r
 217   John Doe <john@example.com>\r
 218   Dr. John Doe <john@example.com>\r
 219   John Doe <JOHN@EXAMPLE.COM>\r
 220   John Doe <john@doe.name>\r
 221   Dr. John Doe <john@doe.name>\r
 222   John Doe <JOHN@doe.name.example.com>\r
 223   John Doe <john@doe.name>\r
 224   Dr. John Doe <john@example.com>\r
 225   Dr. John Doe <john@example.com>\r
 226   Dr. John Doe <john@doe.name>\r
 227   John Doe <john@example.com>\r
 228   John Doe <john@doe.name.example.com>\r
 229 \r
 230 To stir the pool a little more, this could be the output when\r
 231 --duplicate=all (the default) is given.\r
 232 \r
 233 With --duplicate=none the output could be (first match by unique\r
 234 case-insensitive address):\r
 235 \r
 236   John Doe <john@example.com>\r
 237   John Doe <john@doe.name>\r
 238   John Doe <john@doe.name.example.com>\r
 239 \r
 240 (many people may have the same name, but email address is unique per person\r
 241 -- therefore I think 'none' limiting that just to John Doe <john@example.com>\r
 242 would be too little)\r
 243 \r
 244 and with --duplicate=address\r
 245 \r
 246   John Doe <john@example.com>\r
 247   Dr. John Doe <john@example.com>\r
 248   John Doe <john@doe.name>\r
 249   Dr. John Doe <john@doe.name>\r
 250   John Doe <JOHN@doe.name.example.com>\r
 251 \r
 252 (from this output user can choose how the recipient is to be called\r
 253 (like "pseudonyms" mentioned in id:20141010113202.GE28601@TP_L520.localdomain )\r
 254 when sending email)\r
 255 \r
 256 and --duplicate=address-casesensitive\r
 257 \r
 258   John Doe <john@example.com>\r
 259   Dr. John Doe <john@example.com>\r
 260   John Doe <JOHN@EXAMPLE.COM>\r
 261   John Doe <john@doe.name>\r
 262   Dr. John Doe <john@doe.name>\r
 263   John Doe <JOHN@doe.name.example.com>\r
 264   John Doe <john@doe.name.example.com>\r
 265 \r
 266 This reuse of --duplicate was thought out after Jani's irc mention of it.\r
 267 This scheme would leave no room tho the filter-by=name suggestion -- for\r
 268 completeness that would make this look:\r
 269 \r
 270   John Doe <john@example.com>\r
 271   Dr. John Doe <john@example.com>\r
 272 \r
 273 This doesn't look too bad in this particular case but not having ability to\r
 274 see all potential addresses (perhaps the only working address is now\r
 275 hidden) isn't not much for general use. Well, maybe for some specific use\r
 276 --duplicate=no-unique-addresses could be useful :O\r
 277 \r
 278 Ok, this took an hour to get written to (w/ some interruptions). Healthy\r
 279 criticism appreciated :D\r
 280 \r
 281 Tomi\r
 282 \r
 283 // stuff deleted before some 'ADDRESSES' leftover...\r
 284 \r
 285 > @@ -455,18 +542,23 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])\r
 286 >      }\r
 287 >  \r
 288 >      switch (o.output) {\r
 289 > -    default:\r
 290 >      case OUTPUT_SUMMARY:\r
 291 >      case OUTPUT_THREADS:\r
 292 >       ret = do_search_threads (&o);\r
 293 >       break;\r
 294 >      case OUTPUT_MESSAGES:\r
 295 > +    case OUTPUT_SENDER:\r
 296 > +    case OUTPUT_RECIPIENTS:\r
 297 > +    case OUTPUT_ADDRESSES:\r
 298 >      case OUTPUT_FILES:\r
 299 >       ret = do_search_messages (&o);\r
 300 >       break;\r
 301 >      case OUTPUT_TAGS:\r
 302 >       ret = do_search_tags (notmuch, o.format, o.query);\r
 303 >       break;\r
 304 > +    default:\r
 305 > +     fprintf (stderr, "Error: the combination of outputs is not supported.\n");\r
 306 > +     ret = 1;\r
 307 >      }\r
 308 >  \r
 309 >      notmuch_query_destroy (o.query);\r