51/a5d5fe4508b5dfdcc338f208f16e0f6f6f255c

   1 Return-Path: <amdragon@mit.edu>\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 B07B3429E40\r
   6         for <notmuch@notmuchmail.org>; Sat, 21 Jan 2012 16:23:28 -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.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] 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 jlBoRChJsfXw for <notmuch@notmuchmail.org>;\r
  16         Sat, 21 Jan 2012 16:23:28 -0800 (PST)\r
  17 Received: from dmz-mailsec-scanner-5.mit.edu (DMZ-MAILSEC-SCANNER-5.MIT.EDU\r
  18         [18.7.68.34])\r
  19         by olra.theworths.org (Postfix) with ESMTP id DE411431FAF\r
  20         for <notmuch@notmuchmail.org>; Sat, 21 Jan 2012 16:23:27 -0800 (PST)\r
  21 X-AuditID: 12074422-b7fd66d0000008f9-8c-4f1b56ff71e5\r
  22 Received: from mailhub-auth-1.mit.edu ( [18.9.21.35])\r
  23         by dmz-mailsec-scanner-5.mit.edu (Symantec Messaging Gateway) with SMTP\r
  24         id A5.B0.02297.FF65B1F4; Sat, 21 Jan 2012 19:23:27 -0500 (EST)\r
  25 Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
  26         by mailhub-auth-1.mit.edu (8.13.8/8.9.2) with ESMTP id q0M0NQnM018500; \r
  27         Sat, 21 Jan 2012 19:23:27 -0500\r
  28 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  29         (authenticated bits=0)\r
  30         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  31         by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q0M0NPiv024726\r
  32         (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);\r
  33         Sat, 21 Jan 2012 19:23:26 -0500 (EST)\r
  34 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77)\r
  35         (envelope-from <amdragon@MIT.EDU>)\r
  36         id 1RolD3-00007o-Nk; Sat, 21 Jan 2012 19:23:01 -0500\r
  37 Date: Sat, 21 Jan 2012 19:23:01 -0500\r
  38 From: Austin Clements <amdragon@MIT.EDU>\r
  39 To: Peter Feigl <craven@gmx.net>\r
  40 Subject: Re: [PATCH] rewriting notmuch-search for structured output to make\r
  41         other output formats easier\r
  42 Message-ID: <20120122002301.GN16740@mit.edu>\r
  43 References: <1327180568-30385-1-git-send-email-craven@gmx.net>\r
  44         <20120121220407.GK16740@mit.edu> <87obtwlk76.fsf@nexoid.at>\r
  45 MIME-Version: 1.0\r
  46 Content-Type: text/plain; charset=us-ascii\r
  47 Content-Disposition: inline\r
  48 In-Reply-To: <87obtwlk76.fsf@nexoid.at>\r
  49 User-Agent: Mutt/1.5.21 (2010-09-15)\r
  50 X-Brightmail-Tracker:\r
  51  H4sIAAAAAAAAA+NgFupileLIzCtJLcpLzFFi42IR4hRV1v0fJu1vMGWavMXehnZGi+s3ZzI7\r
  52         MHks3rSfzePZqlvMAUxRXDYpqTmZZalF+nYJXBlNV/+yFDToVTy6m9XAeEili5GDQ0LARGLN\r
  53         PL4uRk4gU0ziwr31bF2MXBxCAvsYJSbv2coE4WxglGg6doUdwjnJJPF44Sc2kG4hgSWMEj/K\r
  54         QUwWAVWJ0w3ZIIPYBDQktu1fzghiiwgoSDxb1wRmMwtIS3z73cwEUi4skClxsT0IJMwroCPR\r
  55         0HQTrERIoE5iad8CJoi4oMTJmU9YIFq1JG78ewnWCjJm+T8OkDCngLrE6+1zWUFsUQEViSkn\r
  56         t7FNYBSahaR7FpLuWQjdCxiZVzHKpuRW6eYmZuYUpybrFicn5uWlFuma6uVmluilppRuYgQF\r
  57         M7uL0g7GnweVDjEKcDAq8fAm7JP0F2JNLCuuzD3EKMnBpCTKOzdQ2l+ILyk/pTIjsTgjvqg0\r
  58         J7X4EKMEB7OSCG9ZF1A5b0piZVVqUT5MSpqDRUmcV13rnZ+QQHpiSWp2ampBahFMVoaDQ0mC\r
  59         1x0YtUKCRanpqRVpmTklCGkmDk6Q4TxAw21BaniLCxJzizPTIfKnGHU5vvxuO88oxJKXn5cq\r
  60         Jc4bDVIkAFKUUZoHNweWhF4xigO9JcxrD1LFA0xgcJNeAS1hAlrCkScFsqQkESEl1cC4itc0\r
  61         R/L6345kg63nLky032ju3mMcHqpdYiK05cqdA3sLohY9qr0Vk/Njq8V66c43Ar9rHZgYDZQu\r
  62         LhEMLGwIXHTpic0V8U8cmr8XhL/arDdpG9OU6wwGocUtO+LX2NwvOLWpJlPIPSlAeqFFtLHU\r
  63         mo/y4ekpNjpBhs2cc2zyvHP1DpRd91diKc5INNRiLipOBADAa5ZJHQMAAA==\r
  64 Cc: notmuch@notmuchmail.org\r
  65 X-BeenThere: notmuch@notmuchmail.org\r
  66 X-Mailman-Version: 2.1.13\r
  67 Precedence: list\r
  68 List-Id: "Use and development of the notmuch mail system."\r
  69         <notmuch.notmuchmail.org>\r
  70 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  71         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  72 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  73 List-Post: <mailto:notmuch@notmuchmail.org>\r
  74 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  75 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  76         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  77 X-List-Received-Date: Sun, 22 Jan 2012 00:23:28 -0000\r
  78 \r
  79 Quoth Peter Feigl on Jan 22 at 12:17 am:\r
  80 > On Sat, 21 Jan 2012 17:04:07 -0500, Austin Clements <amdragon@MIT.EDU> wrote:\r
  81 > > Quoth Peter Feigl on Jan 21 at 10:16 pm:\r
  82 > > I think this is a great idea and I'm a fan of having an S-expression\r
  83 > > format, but I also think there's a much easier and more general way to\r
  84 > > structure this.\r
  85 > > \r
  86 > > In particular, I don't think you should hijack search_format, since\r
  87 > > you'll wind up repeating most of this work for anything else that\r
  88 > > needs to output structured data (notmuch show, at least).  Rather, I'd\r
  89 > > suggest creating a general "structure printer" struct that isn't tied\r
  90 > > to search.  You've essentially already done this, you just called it\r
  91 > > search_format.  Then, leave the existing format callbacks in place,\r
  92 > > just use this new API instead of lots of printf calls.\r
  93 > \r
  94 > I'm sorry I haven't been more clear about this, the intention was all\r
  95 > along to check whether this would be ok in notmuch-search, and if it got\r
  96 > accepted there, to factor it out into a separate file and then use it in\r
  97 > notmuch-show and notmuch-reply. There's nothing in the printer (except\r
  98 > for the name of the struct) that ties it to search.\r
  99 \r
 100 Great!  However, it seems counterproductive to put all of these\r
 101 structures and functions into search only to then move them somewhere\r
 102 else.  Wouldn't it make more sense to introduce the generic structure\r
 103 printer in a first patch, then refactor the existing code to use it in\r
 104 a second (or maybe more) patch in a series?\r
 105 \r
 106 > > What about all of those annoying {tag,item}_{start,sep,end} fields?  I\r
 107 > > think you can simultaneously get rid of those *and* simplify the\r
 108 > > structure printer API.  If the structure printer is allowed to keep a\r
 109 > > little state, it can automatically insert separators.  With a little\r
 110 > > nesting state, it could even insert terminators by just saying "pop me\r
 111 > > to this level".  This could probably be better, but I'm imagining\r
 112 > > something like\r
 113 > \r
 114 > I agree, however this is complicated by the fact that there are\r
 115 > additional restrictions on the actually printed code: newlines should be\r
 116 > placed at strategic locations to improve parsability, which could be\r
 117 > hard to decide in the printer alone without support from the logic that\r
 118 > drives it.\r
 119 \r
 120 True, but that support is easy to add and, I would argue, the exact\r
 121 implementation of framing *should* be up to the printer (though\r
 122 obviously where to place the framing should be up to the caller).\r
 123 Following the general structure of the API I was proposing, you could\r
 124 add a function like\r
 125 \r
 126 /* Print a framing break that is easy to detect in a parser. */\r
 127 void\r
 128 sprinter_break (struct sprinter *sp);\r
 129 \r
 130 that for JSON and S-expressions could just print a newline.  Or, for\r
 131 JSON, if we want separators to appear before the newline (which they\r
 132 don't have to; we can choose our framing however we want), it could\r
 133 simply record that a newline should be printed after the next\r
 134 separator or terminator.\r
 135 \r
 136 > > struct sprinter *\r
 137 > > new_json_sprinter (const void *ctx, FILE *stream);\r
 138 > > struct sprinter *\r
 139 > > new_sexp_sprinter (const void *ctx, FILE *stream);\r
 140 > > \r
 141 > > /* Start a map (a JSON object or a S-expression alist/plist/whatever)\r
 142 > >  * and return the nesting level of the map. */\r
 143 > > int\r
 144 > > sprinter_map (struct sprinter *sp);\r
 145 > > /* Start a list (aka array) and return the nesting level of the list. */\r
 146 > > int\r
 147 > > sprinter_list (struct sprinter *sp);\r
 148 > > \r
 149 > > /* Close maps and lists until reaching level. */\r
 150 > > void\r
 151 > > sprinter_pop (struct sprinter *sp, int level);\r
 152 > > \r
 153 > > void\r
 154 > > sprinter_map_key (struct sprinter *sp, const char *key);\r
 155 > > void\r
 156 > > sprinter_number (struct sprinter *sp, int val);\r
 157 > > void\r
 158 > > sprinter_string (struct sprinter *sp, const char *val);\r
 159 > > void\r
 160 > > sprinter_bool (struct sprinter *sp, notmuch_bool_t val);\r
 161 > > \r
 162 > > and that's it.  This would also subsume your format_attribute_*\r
 163 > > helpers.\r
 164 > > \r
 165 > > Unfortunately, it's a pain to pass things like a structure printer\r
 166 > > object around formatters (too bad notmuch isn't written in C++, eh?).\r
 167 > > I think it's better to address this than to structure around it.\r
 168 > > Probably the simplest thing to do is to make a struct for formatter\r
 169 > > state and pass that in to the callbacks.  You could also more\r
 170 > > completely emulate classes, but that would probably be overkill for\r
 171 > > this.\r
 172 > \r
 173 > I believe this approach is similar to the one I've implemented (though\r
 174 > probably higher level, not so many details explicitly written into the\r
 175 > formatting code). I would suggest trying to get any sort of structured\r
 176 \r
 177 *nods* It was definitely inspired by yours.  The complexity has to go\r
 178 somewhere, and in the long run it seems it would be better to isolate\r
 179 it in the printer than to deal with it in every caller.  My ulterior\r
 180 motive was to maintain roughly the existing and extensible callback\r
 181 structure while eliminating the need for the various start/sep/end\r
 182 fields, which would have to differ between the formats and would\r
 183 otherwise duplicate knowledge that should be isolated to a structure\r
 184 printer.\r
 185 \r
 186 > formatters (whether more like your suggestions or like the thing I\r
 187 > implemented doesn't matter so much) into the main codebase, and then\r
 188 > refactoring the other parts to use it. I've thought about providing a\r
 189 > single patch to all of notmuch that accomplishes this, but I've deemed\r
 190 > it too large and complicated to be accepted, I thought limiting it to\r
 191 > notmuch-search would be a way to get it set up, so that it could be\r
 192 > expanded to the other parts later.\r
 193 \r
 194 I've found that smaller patches are much more likely to get reviewed\r
 195 on the notmuch list, even if they're part of a series that adds up to\r
 196 a big change.\r
 197 \r
 198 > Thanks for the comments, I'll keep thinking about your design, a very\r
 199 > interesting idea!\r
 200 > \r
 201 > Peter\r