59/430c98b92839f3db0dbfeb5e43b525be80ffbc

   1 Return-Path: <jrollins@finestructure.net>\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 8D113431FAF\r
   6         for <notmuch@notmuchmail.org>; Sat,  3 Mar 2012 14:06:05 -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.29\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-2.29 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_MED=-2.3, T_MIME_NO_TEXT=0.01] 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 i14ZJaFfndvm for <notmuch@notmuchmail.org>;\r
  16         Sat,  3 Mar 2012 14:06:05 -0800 (PST)\r
  17 Received: from outgoing-mail.its.caltech.edu (outgoing-mail.its.caltech.edu\r
  18         [131.215.239.19])\r
  19         by olra.theworths.org (Postfix) with ESMTP id D1ADC431FAE\r
  20         for <notmuch@notmuchmail.org>; Sat,  3 Mar 2012 14:06:04 -0800 (PST)\r
  21 Received: from fire-doxen.imss.caltech.edu (localhost [127.0.0.1])\r
  22         by fire-doxen-postvirus (Postfix) with ESMTP id 2DEB2328041;\r
  23         Sat,  3 Mar 2012 14:06:04 -0800 (PST)\r
  24 X-Spam-Scanned: at Caltech-IMSS on fire-doxen by amavisd-new\r
  25 Received: from finestructure.net (DHCP-123-180.caltech.edu [131.215.123.180])\r
  26         (Authenticated sender: jrollins)\r
  27         by fire-doxen-submit (Postfix) with ESMTP id 3B221328005;\r
  28         Sat,  3 Mar 2012 14:06:01 -0800 (PST)\r
  29 Received: by finestructure.net (Postfix, from userid 1000)\r
  30         id 07B821276; Sat,  3 Mar 2012 14:06:00 -0800 (PST)\r
  31 From: Jameson Graef Rollins <jrollins@finestructure.net>\r
  32 To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org\r
  33 Subject: Re: [PATCH 5/5] show: Convert raw format to the new self-recursive\r
  34         style\r
  35 In-Reply-To: <1330752025-2542-6-git-send-email-amdragon@mit.edu>\r
  36 References: <1330752025-2542-1-git-send-email-amdragon@mit.edu>\r
  37         <1330752025-2542-6-git-send-email-amdragon@mit.edu>\r
  38 User-Agent: Notmuch/0.11.1+264~gb8fb66b (http://notmuchmail.org) Emacs/23.3.1\r
  39         (x86_64-pc-linux-gnu)\r
  40 Date: Sat, 03 Mar 2012 14:05:58 -0800\r
  41 Message-ID: <87zkbxqr09.fsf@servo.finestructure.net>\r
  42 MIME-Version: 1.0\r
  43 Content-Type: multipart/signed; boundary="=-=-=";\r
  44         micalg=pgp-sha256; protocol="application/pgp-signature"\r
  45 X-BeenThere: notmuch@notmuchmail.org\r
  46 X-Mailman-Version: 2.1.13\r
  47 Precedence: list\r
  48 List-Id: "Use and development of the notmuch mail system."\r
  49         <notmuch.notmuchmail.org>\r
  50 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  51         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  52 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  53 List-Post: <mailto:notmuch@notmuchmail.org>\r
  54 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  55 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  56         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  57 X-List-Received-Date: Sat, 03 Mar 2012 22:06:05 -0000\r
  58 \r
  59 --=-=-=\r
  60 Content-Transfer-Encoding: quoted-printable\r
  61 \r
  62 Hey, Austin.  As always, thank you so much for your hard work on this\r
  63 rewrite.  It looks like things are definitely moving the right\r
  64 direction.\r
  65 \r
  66 I haven't done a full review of this patch set, and I've been pretty out\r
  67 of the loop on this stuff recently, but I do notice that there are some\r
  68 changes to the tests that don't look right to me.\r
  69 \r
  70 On Sat,  3 Mar 2012 00:20:25 -0500, Austin Clements <amdragon@MIT.EDU> wrot=\r
  71 e:\r
  72 > This is fully compatible for root and leaf parts, but drops support\r
  73 > for interior parts.  Showing interior parts in raw has always been\r
  74 > braindead broken, so I don't think anyone will miss this.  Tests have\r
  75 > been updated to reflect this.\r
  76 \r
  77 I think I'm confused about this "drop support for interior parts".  What\r
  78 constitutes an "interior part"?  Aren't all parts interior?  It looks\r
  79 From=20the patch that maybe you're referring specifically to rfc822 parts?\r
  80 \r
  81 I can understand not supporting output of multipart parts in raw; it's\r
  82 unclear what a "raw" multipart even is.  But I think we do need to\r
  83 support common-sense handling of rfc822 parts, even if it requires\r
  84 special casing.  These following two test modifications illustrate the\r
  85 issue:\r
  86 \r
  87 >  test_begin_subtest "--format=3Draw --part=3D3, rfc822 part"\r
  88 > -test_subtest_known_broken\r
  89 > -\r
  90 > -notmuch show --format=3Draw --part=3D3 'id:87liy5ap00.fsf@yoom.home.cwor=\r
  91 th.org' >OUTPUT\r
  92 > -test_expect_equal_file OUTPUT embedded_message\r
  93 > +notmuch show --format=3Draw --part=3D3 'id:87liy5ap00.fsf@yoom.home.cwor=\r
  94 th.org' >&OUTPUT\r
  95 > +cat <<EOF >EXPECTED\r
  96 > +Error: Raw only supports root and leaf parts\r
  97 > +EOF\r
  98 > +test_expect_equal_file OUTPUT EXPECTED\r
  99 \r
 100 I pretty strongly think that this test needs to remain how it was.  If\r
 101 someone forwards me a message as a rfc822 part I should be able to\r
 102 retrieve the full forwarded message directly, by e.g. redirecting it to\r
 103 a file and recreating the original message exactly intact.  That's why I\r
 104 constructed this test the way I did originally, and left it\r
 105 known_broken.  If we can't support this now that's fine, but I still\r
 106 think this test describes an important needed functionality that we\r
 107 should strive to support at some point.  Maybe it needs an entirely new\r
 108 output formatter, or some special casing, but I still think it's\r
 109 reasonable to expect that we should support this.\r
 110 \r
 111 >  test_begin_subtest "--format=3Draw --part=3D4, rfc822's html part"\r
 112 > -notmuch show --format=3Draw --part=3D4 'id:87liy5ap00.fsf@yoom.home.cwor=\r
 113 th.org' >OUTPUT\r
 114 > +notmuch show --format=3Draw --part=3D4 'id:87liy5ap00.fsf@yoom.home.cwor=\r
 115 th.org' >&OUTPUT\r
 116 >  cat <<EOF >EXPECTED\r
 117 > -<p>This is an embedded message, with a multipart/alternative part.</p>\r
 118 > -This is an embedded message, with a multipart/alternative part.\r
 119 > +Error: Raw only supports root and leaf parts\r
 120 >  EOF\r
 121 >  test_expect_equal_file OUTPUT EXPECTED\r
 122 \r
 123 Maybe this is ultimately a limitation of what we can expect the raw\r
 124 formatter to do, but isn't this a leaf part?  As with whole rfc822\r
 125 parts, I also expect that we should be able to retrieve interior leaf\r
 126 parts of rfc822 message parts.  So I think I would prefer that this test\r
 127 just move to test_subtest_known_broken until a better way to handle this\r
 128 is figured out.\r
 129 \r
 130 Without looking into the details of your whole show rewrite, I'm\r
 131 guessing that we just can't reasonably expect the raw formatter to\r
 132 handle rfc822 parts the way we need.  Is that correct?  Is there some\r
 133 other formatter that might be better suited for this?  Like I say, I\r
 134 think it's ok if we have to have some special casing for this particular\r
 135 type of part.  But either way I think we should try to support handling\r
 136 rfc822 parts in a useful way.\r
 137 \r
 138 jamie.\r
 139 \r
 140 --=-=-=\r
 141 Content-Type: application/pgp-signature\r
 142 \r
 143 -----BEGIN PGP SIGNATURE-----\r
 144 Version: GnuPG v1.4.11 (GNU/Linux)\r
 145 \r
 146 iQIcBAEBCAAGBQJPUpXGAAoJEO00zqvie6q8SxYP/3q6IAbQc9OSzTNiGRzvjo/k\r
 147 posFVhzBrvI4BsgFWK4buYwyOKtP2YQGp4RuN047iRpKoxL3X6+R1GQk1avCx8Ov\r
 148 2g89P03Hv+LPcXVyQ65wk7zGwIrlSBtDGR4NsTXg7LXs35PX368m+WeiiFBZmA8S\r
 149 bcDllCUuDe49HNqX/CVOYuxksC+OTdIEUnuPwnE1Ahi+mVOxmkqFyzdapfAn3BKd\r
 150 tywqmzrZzJ5tTo2Nepgl5q7bWnXvpZ7vpW6xHbRMWojZuYtn7D2UAgfLordI57ih\r
 151 xI9cesrc7omysPjWB6pk+7sDX1t25f9q1yxyLnlkUK0CobkrGcA2IQTNVUIieEI/\r
 152 ZBU28znNsyCMH+WIFL8xkCpSzNUn5wduwvtpTUIkPUEQ4jn3tkXR1Vr2VKgLRPgp\r
 153 jSyRtEnS3Wg0l8nmB+QK7u7ljZE6yd3XbjfQj0D+D0ZJVuxtzeKHUKK+uOwQK+Tb\r
 154 JMtVlVXwMF4dHEG4uvADvklXdI4+vhEKtxVEoTxhgK5rO0ttFReQ27u47e2twBGp\r
 155 I+n2Jdp6tuRftcqe+hwXo7laj/ShRjE7I0rksejqCLEKv3EmuquNAEFtcU0ioDO6\r
 156 bV6Clkk+26QcwkP8PMgdpyiEsW5zvTUZBc0aSW+N39/8PzbJ+bXYmrZZIw3De/eA\r
 157 gXj99B/CYR/QXg8AlY/P\r
 158 =cwLz\r
 159 -----END PGP SIGNATURE-----\r
 160 --=-=-=--\r