a8/9a041ab202158faf08f0472c65400dfef161f4

   1 Return-Path: <sojkam1@fel.cvut.cz>\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 6596949F192\r
   6         for <notmuch@notmuchmail.org>; Thu, 11 Mar 2010 04:46:11 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -2.252\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-2.252 tagged_above=-999 required=5 tests=[AWL=0.347,\r
  12         BAYES_00=-2.599] autolearn=unavailable\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 LY56eQiZBH3O for <notmuch@notmuchmail.org>;\r
  16         Thu, 11 Mar 2010 04:46:07 -0800 (PST)\r
  17 Received: from max.feld.cvut.cz (max.feld.cvut.cz [147.32.192.36])\r
  18         by olra.theworths.org (Postfix) with ESMTP id 68E7849F191\r
  19         for <notmuch@notmuchmail.org>; Thu, 11 Mar 2010 04:46:07 -0800 (PST)\r
  20 Received: from localhost (unknown [192.168.200.4])\r
  21         by max.feld.cvut.cz (Postfix) with ESMTP id 6358119F35EB;\r
  22         Thu, 11 Mar 2010 13:45:48 +0100 (CET)\r
  23 X-Virus-Scanned: IMAP AMAVIS\r
  24 Received: from max.feld.cvut.cz ([192.168.200.1])\r
  25         by localhost (styx.feld.cvut.cz [192.168.200.4]) (amavisd-new,\r
  26         port 10044)\r
  27         with ESMTP id oD2Q5jdi-E8Y; Thu, 11 Mar 2010 13:45:46 +0100 (CET)\r
  28 Received: from imap.feld.cvut.cz (imap.feld.cvut.cz [147.32.192.34])\r
  29         by max.feld.cvut.cz (Postfix) with ESMTP id E433B19F35E7;\r
  30         Thu, 11 Mar 2010 13:45:45 +0100 (CET)\r
  31 Received: from steelpick.localdomain (k335-30.felk.cvut.cz [147.32.86.30])\r
  32         (Authenticated sender: sojkam1)\r
  33         by imap.feld.cvut.cz (Postfix) with ESMTPSA id AAA97FA003;\r
  34         Thu, 11 Mar 2010 13:45:45 +0100 (CET)\r
  35 Received: from wsh by steelpick.localdomain with local (Exim 4.71)\r
  36         (envelope-from <sojkam1@fel.cvut.cz>)\r
  37         id 1Nphlp-00071H-HC; Thu, 11 Mar 2010 13:45:45 +0100\r
  38 From: Michal Sojka <sojkam1@fel.cvut.cz>\r
  39 To: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>, cworth@cworth.org\r
  40 In-Reply-To:\r
  41  <1268238686-13605-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>\r
  42 References: <87zl2hic7d.fsf@yoom.home.cworth.org>\r
  43         <1268238686-13605-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>\r
  44 Date: Thu, 11 Mar 2010 13:45:45 +0100\r
  45 Message-ID: <87r5nrm3gm.fsf@steelpick.localdomain>\r
  46 MIME-Version: 1.0\r
  47 Content-Type: text/plain; charset=us-ascii\r
  48 Cc: "Aneesh Kumar K.V" <aneesh.kumar@gmail.com>, notmuch@notmuchmail.org\r
  49 Subject: Re: [notmuch] [PATCH -V3 1/2] notmuch-reply: Add support for\r
  50         replying only to sender\r
  51 X-BeenThere: notmuch@notmuchmail.org\r
  52 X-Mailman-Version: 2.1.13\r
  53 Precedence: list\r
  54 List-Id: "Use and development of the notmuch mail system."\r
  55         <notmuch.notmuchmail.org>\r
  56 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  57         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  58 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  59 List-Post: <mailto:notmuch@notmuchmail.org>\r
  60 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  61 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  62         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  63 X-List-Received-Date: Thu, 11 Mar 2010 12:46:11 -0000\r
  64 \r
  65 Hi,\r
  66 \r
  67 On Wed, 10 Mar 2010, Aneesh Kumar K.V wrote:\r
  68 > From: Aneesh Kumar K.V <aneesh.kumar@gmail.com>\r
  69 > \r
  70 > This patch add --recipient=all|sender option\r
  71 > \r
  72 > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@gmail.com>\r
  73 > ---\r
  74 >  notmuch-client.h |    2 +\r
  75 >  notmuch-reply.c  |   55 ++++++++++++++++++++++++++++++++++++++++-------------\r
  76 >  2 files changed, 43 insertions(+), 14 deletions(-)\r
  77 > \r
  78 > diff --git a/notmuch-client.h b/notmuch-client.h\r
  79 > index c80b39c..26fdb4a 100644\r
  80 > --- a/notmuch-client.h\r
  81 > +++ b/notmuch-client.h\r
  82 > @@ -70,6 +70,8 @@\r
  83 >  #define STRNCMP_LITERAL(var, literal) \\r
  84 >      strncmp ((var), (literal), sizeof (literal) - 1)\r
  85 >  \r
  86 > +#define NOTMUCH_REPLY_ALL   0x1\r
  87 > +#define NOTMUCH_REPLY_SENDER_ONLY 0x2\r
  88 \r
  89 Why not to define this as enum? When I see a definition like this\r
  90 it reminds me bit flags which can be used together e.g.\r
  91   (NOTMUCH_REPLY_ALL | NOTMUCH_REPLY_SENDER_ONLY).\r
  92 \r
  93 This has obviously no sense here.\r
  94 \r
  95 >  static inline void\r
  96 >  chomp_newline (char *str)\r
  97 >  {\r
  98 > diff --git a/notmuch-reply.c b/notmuch-reply.c\r
  99 > index 6c15536..e8a0820 100644\r
 100 > --- a/notmuch-reply.c\r
 101 > +++ b/notmuch-reply.c\r
 102 > @@ -232,20 +232,37 @@ reply_to_header_is_redundant (notmuch_message_t *message)\r
 103 >  static const char *\r
 104 >  add_recipients_from_message (GMimeMessage *reply,\r
 105 >                            notmuch_config_t *config,\r
 106 > -                          notmuch_message_t *message)\r
 107 > +                          notmuch_message_t *message,\r
 108 > +                          int reply_options)\r
 109 >  {\r
 110 > -    struct {\r
 111 > +    struct reply_to_map {\r
 112 >       const char *header;\r
 113 >       const char *fallback;\r
 114 >       GMimeRecipientType recipient_type;\r
 115 > -    } reply_to_map[] = {\r
 116 > +    } ;\r
 117 > +    const char *from_addr = NULL;\r
 118 > +    unsigned int i;\r
 119 > +    struct reply_to_map *reply_to_map;\r
 120 > +\r
 121 > +    struct reply_to_map reply_to_map_all[] = {\r
 122 >       { "reply-to", "from", GMIME_RECIPIENT_TYPE_TO  },\r
 123 >       { "to",         NULL, GMIME_RECIPIENT_TYPE_TO  },\r
 124 >       { "cc",         NULL, GMIME_RECIPIENT_TYPE_CC  },\r
 125 > -     { "bcc",        NULL, GMIME_RECIPIENT_TYPE_BCC }\r
 126 > +     { "bcc",        NULL, GMIME_RECIPIENT_TYPE_BCC },\r
 127 > +     {  NULL,        NULL, 0 }\r
 128 >      };\r
 129 > -    const char *from_addr = NULL;\r
 130 > -    unsigned int i;\r
 131 > +\r
 132 > +    /* we try from first and then reply-to */\r
 133 > +    struct reply_to_map reply_to_map_sender[] = {\r
 134 > +     { "from", "reply-to", GMIME_RECIPIENT_TYPE_TO  },\r
 135 > +     {  NULL,        NULL, 0 }\r
 136 > +    };\r
 137 \r
 138 I'm not sure whether an e-mail without From: is a valid e-mail. If not,\r
 139 you would never use "reply-to" header. Also, given the link below\r
 140 (http://www.unicom.com/pw/reply-to-harmful.html), this will not behave\r
 141 as Carl likes :). Finally, don't forget that reply_to_map can be\r
 142 modified in add_recipients_from_message().\r
 143 \r
 144 I missed your original patch so I implement this for myself in a less\r
 145 intrusive way (see\r
 146 id:1267464588-21050-1-git-send-email-sojkam1@fel.cvut.cz). What I like\r
 147 at your approach is that --recipient option can have several values. I\r
 148 think it would be useful to have three possible values of this option:\r
 149 "all", "sender" and something like "reply-to-or-sender". The latter\r
 150 option would cause using of Reply-to: header event if it is found as\r
 151 redundant by reply_to_header_is_redundant(). I find this useful for\r
 152 several private mailing lists I use.\r
 153 \r
 154 -Michal\r
 155 > +\r
 156 > +    if (reply_options == NOTMUCH_REPLY_SENDER_ONLY) {\r
 157 > +     reply_to_map = reply_to_map_sender;\r
 158 > +    } else {\r
 159 > +     reply_to_map = reply_to_map_all;\r
 160 > +    }\r
 161 >  \r
 162 >      /* Some mailing lists munge the Reply-To header despite it being A Bad\r
 163 >       * Thing, see http://www.unicom.com/pw/reply-to-harmful.html\r