53/2ee6448c387350595817b6dbc136cfaf6d9b38

   1 Return-Path: <novalazy@gmail.com>\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 53828431FAF\r
   6         for <notmuch@notmuchmail.org>; Wed, 23 Jan 2013 00:04:33 -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: -0.799\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.799 tagged_above=-999 required=5\r
  12         tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1,\r
  13         FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  14 Received: from olra.theworths.org ([127.0.0.1])\r
  15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  16         with ESMTP id KOcRMQ7IBIqg for <notmuch@notmuchmail.org>;\r
  17         Wed, 23 Jan 2013 00:04:32 -0800 (PST)\r
  18 Received: from mail-pa0-f42.google.com (mail-pa0-f42.google.com\r
  19         [209.85.220.42]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id 87B5C431FAE\r
  22         for <notmuch@notmuchmail.org>; Wed, 23 Jan 2013 00:04:32 -0800 (PST)\r
  23 Received: by mail-pa0-f42.google.com with SMTP id rl6so4637742pac.1\r
  24         for <notmuch@notmuchmail.org>; Wed, 23 Jan 2013 00:04:30 -0800 (PST)\r
  25 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;\r
  26         h=x-received:date:message-id:from:to:cc:subject:in-reply-to\r
  27         :references:mime-version:content-type:content-disposition\r
  28         :content-transfer-encoding;\r
  29         bh=CX72w5YPg+L5328OWEQqhlhCHBu8scxfrZ9EQdPSS0Y=;\r
  30         b=xJ4E4IRl/vpYHODtNKCDZiPLIhY0IIlUUCj3a3WQqBPY9YHruwfJM1Ir+KBU71YLIj\r
  31         e4E8xj9up9Wmlj9kbHC5RIMEg8RxWQk07l786YXcQdAvZ3P8+f53KdMuNqPjnNouo+QW\r
  32         30QS6Xuk+dav6eYUBg/BQazNnk+zCdprqxDztnkaSqOmaDqUgRzzO2eZzp6kAlJtUIRq\r
  33         myxmbn2eOSbBgjc7lis4fk3EcN1UFEDHpAKfP5YNhaWk/EGWQZWxnO7IvksNCW0cTYwV\r
  34         9GohcBr27gOpC5kVa8RHqblYcTAlNBNv3iKmEQczE6yD1MosQgJAI0ewQQsUWmlxOPkh\r
  35         uzAg==\r
  36 X-Received: by 10.66.76.37 with SMTP id h5mr2381731paw.33.1358928270668;\r
  37         Wed, 23 Jan 2013 00:04:30 -0800 (PST)\r
  38 Received: from localhost (215.42.233.220.static.exetel.com.au.\r
  39         [220.233.42.215])\r
  40         by mx.google.com with ESMTPS id qn3sm12357330pbb.56.2013.01.23.00.04.27\r
  41         (version=TLSv1.2 cipher=RC4-SHA bits=128/128);\r
  42         Wed, 23 Jan 2013 00:04:29 -0800 (PST)\r
  43 Date: Wed, 23 Jan 2013 19:04:24 +1100\r
  44 Message-ID: <20130123190424.GA1980@hili.localdomain>\r
  45 From: Peter Wang <novalazy@gmail.com>\r
  46 To: Jani Nikula <jani@nikula.org>\r
  47 Subject: Re: [PATCH v3 01/20] cli: add stub for insert command\r
  48 In-Reply-To: <87vcaoj3i0.fsf@nikula.org>\r
  49 References: <1358643004-14522-1-git-send-email-novalazy@gmail.com>\r
  50         <1358643004-14522-2-git-send-email-novalazy@gmail.com>\r
  51         <87vcaoj3i0.fsf@nikula.org>\r
  52 MIME-Version: 1.0\r
  53 Content-Type: text/plain; charset=utf-8\r
  54 Content-Disposition: inline\r
  55 Content-Transfer-Encoding: 8bit\r
  56 Cc: notmuch@notmuchmail.org\r
  57 X-BeenThere: notmuch@notmuchmail.org\r
  58 X-Mailman-Version: 2.1.13\r
  59 Precedence: list\r
  60 List-Id: "Use and development of the notmuch mail system."\r
  61         <notmuch.notmuchmail.org>\r
  62 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  63         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  64 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  65 List-Post: <mailto:notmuch@notmuchmail.org>\r
  66 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  67 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  68         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  69 X-List-Received-Date: Wed, 23 Jan 2013 08:04:33 -0000\r
  70 \r
  71 On Tue, 22 Jan 2013 23:45:43 +0200, Jani Nikula <jani@nikula.org> wrote:\r
  72 > > Though it could be used as an alternative to notmuch new, the reason\r
  73 > > I want this is to allow my notmuch frontend to add postponed or sent\r
  74 > > messages to the mail store and notmuch database, without resorting to\r
  75 > > another tool (e.g. notmuch-deliver) nor directly modifying the maildir.\r
  76 > \r
  77 > This review is based on the following patches squashed together:\r
  78 > \r
  79 >       cli: add stub for insert command\r
  80 >       insert: open Maildir tmp file\r
  81 >       insert: copy stdin to Maildir tmp file\r
  82 >       insert: move file from Maildir tmp to new\r
  83 >       insert: add new message to database\r
  84 >       insert: apply default tags to new message\r
  85 >       insert: parse and apply command-line tag operations\r
  86 >       insert: fsync after writing tmp file\r
  87 >       insert: trap SIGINT and clean up\r
  88 >       insert: add copyright line from notmuch-deliver\r
  89 > \r
  90 > It's much easier for me to grasp the big picture this way.\r
  91 > \r
  92 \r
  93 So there *is* a limit to how fine-grained notmuchers want their patches ;-)\r
  94 \r
  95 > > +static notmuch_bool_t\r
  96 > > +maildir_move_tmp_to_new (const char *tmppath, const char *newpath)\r
  97 > > +{\r
  98 > > +    if (rename (tmppath, newpath) != 0) {\r
  99 > > +   fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));\r
 100 > > +   return FALSE;\r
 101 > > +    }\r
 102 > > +\r
 103 > > +    return TRUE;\r
 104 > > +}\r
 105 > \r
 106 > IMO you could just use rename() inline in the caller, without a wrapper.\r
 107 \r
 108 A subsequent patch fsyncs the directory here.\r
 109 \r
 110 > > +/* Copy the contents of standard input (fdin) into fdout. */\r
 111 > > +static notmuch_bool_t\r
 112 > > +copy_stdin (int fdin, int fdout)\r
 113 > \r
 114 > The comment and the function name imply the function has something to do\r
 115 > with stdin, while it only cares about file descriptors.\r
 116 \r
 117 Tomi pointed out that the error message refers to standard input.\r
 118 \r
 119 > > +/* Add the specified message file to the notmuch database, applying tags.\r
 120 > > + * The file is renamed to encode notmuch tags as maildir flags. */\r
 121 > > +static notmuch_bool_t\r
 122 > > +add_file_to_database (notmuch_database_t *notmuch, const char *path,\r
 123 > > +                 tag_op_list_t *tag_ops)\r
 124 > > +{\r
 125 > > +    notmuch_message_t *message;\r
 126 > > +    notmuch_status_t status;\r
 127 > > +\r
 128 > > +    status = notmuch_database_add_message (notmuch, path, &message);\r
 129 > > +    switch (status) {\r
 130 > > +    case NOTMUCH_STATUS_SUCCESS:\r
 131 > > +   break;\r
 132 > > +    case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:\r
 133 > > +   fprintf (stderr, "Warning: duplicate message.\n");\r
 134 > \r
 135 > This is not uncommon. Why the warning?\r
 136 > \r
 137 \r
 138 I put in the warning because I wasn't sure, then forgot to revisit it.\r
 139 \r
 140 > Also, notmuch new does not apply new.tags in this case. Are you sure we\r
 141 > want to do that here? (You get mail, you read and archive it, you get\r
 142 > the dupe, it pops up unread in your inbox again.)\r
 143 \r
 144 Should command-line tags to be applied to duplicate messages?\r
 145 I'm thinking not.\r
 146 \r
 147 I'll fix the other problems you found.\r
 148 \r
 149 Peter\r