From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 8D113431FAF for ; Sat, 3 Mar 2012 14:06:05 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -2.29 X-Spam-Level: X-Spam-Status: No, score=-2.29 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_MED=-2.3, T_MIME_NO_TEXT=0.01] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id i14ZJaFfndvm for ; Sat, 3 Mar 2012 14:06:05 -0800 (PST) Received: from outgoing-mail.its.caltech.edu (outgoing-mail.its.caltech.edu [131.215.239.19]) by olra.theworths.org (Postfix) with ESMTP id D1ADC431FAE for ; Sat, 3 Mar 2012 14:06:04 -0800 (PST) Received: from fire-doxen.imss.caltech.edu (localhost [127.0.0.1]) by fire-doxen-postvirus (Postfix) with ESMTP id 2DEB2328041; Sat, 3 Mar 2012 14:06:04 -0800 (PST) X-Spam-Scanned: at Caltech-IMSS on fire-doxen by amavisd-new Received: from finestructure.net (DHCP-123-180.caltech.edu [131.215.123.180]) (Authenticated sender: jrollins) by fire-doxen-submit (Postfix) with ESMTP id 3B221328005; Sat, 3 Mar 2012 14:06:01 -0800 (PST) Received: by finestructure.net (Postfix, from userid 1000) id 07B821276; Sat, 3 Mar 2012 14:06:00 -0800 (PST) From: Jameson Graef Rollins To: Austin Clements , notmuch@notmuchmail.org Subject: Re: [PATCH 5/5] show: Convert raw format to the new self-recursive style In-Reply-To: <1330752025-2542-6-git-send-email-amdragon@mit.edu> References: <1330752025-2542-1-git-send-email-amdragon@mit.edu> <1330752025-2542-6-git-send-email-amdragon@mit.edu> User-Agent: Notmuch/0.11.1+264~gb8fb66b (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) Date: Sat, 03 Mar 2012 14:05:58 -0800 Message-ID: <87zkbxqr09.fsf@servo.finestructure.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 03 Mar 2012 22:06:05 -0000 --=-=-= Content-Transfer-Encoding: quoted-printable Hey, Austin. As always, thank you so much for your hard work on this rewrite. It looks like things are definitely moving the right direction. I haven't done a full review of this patch set, and I've been pretty out of the loop on this stuff recently, but I do notice that there are some changes to the tests that don't look right to me. On Sat, 3 Mar 2012 00:20:25 -0500, Austin Clements wrot= e: > This is fully compatible for root and leaf parts, but drops support > for interior parts. Showing interior parts in raw has always been > braindead broken, so I don't think anyone will miss this. Tests have > been updated to reflect this. I think I'm confused about this "drop support for interior parts". What constitutes an "interior part"? Aren't all parts interior? It looks From=20the patch that maybe you're referring specifically to rfc822 parts? I can understand not supporting output of multipart parts in raw; it's unclear what a "raw" multipart even is. But I think we do need to support common-sense handling of rfc822 parts, even if it requires special casing. These following two test modifications illustrate the issue: > test_begin_subtest "--format=3Draw --part=3D3, rfc822 part" > -test_subtest_known_broken > - > -notmuch show --format=3Draw --part=3D3 'id:87liy5ap00.fsf@yoom.home.cwor= th.org' >OUTPUT > -test_expect_equal_file OUTPUT embedded_message > +notmuch show --format=3Draw --part=3D3 'id:87liy5ap00.fsf@yoom.home.cwor= th.org' >&OUTPUT > +cat <EXPECTED > +Error: Raw only supports root and leaf parts > +EOF > +test_expect_equal_file OUTPUT EXPECTED I pretty strongly think that this test needs to remain how it was. If someone forwards me a message as a rfc822 part I should be able to retrieve the full forwarded message directly, by e.g. redirecting it to a file and recreating the original message exactly intact. That's why I constructed this test the way I did originally, and left it known_broken. If we can't support this now that's fine, but I still think this test describes an important needed functionality that we should strive to support at some point. Maybe it needs an entirely new output formatter, or some special casing, but I still think it's reasonable to expect that we should support this. > test_begin_subtest "--format=3Draw --part=3D4, rfc822's html part" > -notmuch show --format=3Draw --part=3D4 'id:87liy5ap00.fsf@yoom.home.cwor= th.org' >OUTPUT > +notmuch show --format=3Draw --part=3D4 'id:87liy5ap00.fsf@yoom.home.cwor= th.org' >&OUTPUT > cat <EXPECTED > -

This is an embedded message, with a multipart/alternative part.

> -This is an embedded message, with a multipart/alternative part. > +Error: Raw only supports root and leaf parts > EOF > test_expect_equal_file OUTPUT EXPECTED Maybe this is ultimately a limitation of what we can expect the raw formatter to do, but isn't this a leaf part? As with whole rfc822 parts, I also expect that we should be able to retrieve interior leaf parts of rfc822 message parts. So I think I would prefer that this test just move to test_subtest_known_broken until a better way to handle this is figured out. Without looking into the details of your whole show rewrite, I'm guessing that we just can't reasonably expect the raw formatter to handle rfc822 parts the way we need. Is that correct? Is there some other formatter that might be better suited for this? Like I say, I think it's ok if we have to have some special casing for this particular type of part. But either way I think we should try to support handling rfc822 parts in a useful way. jamie. --=-=-= Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iQIcBAEBCAAGBQJPUpXGAAoJEO00zqvie6q8SxYP/3q6IAbQc9OSzTNiGRzvjo/k posFVhzBrvI4BsgFWK4buYwyOKtP2YQGp4RuN047iRpKoxL3X6+R1GQk1avCx8Ov 2g89P03Hv+LPcXVyQ65wk7zGwIrlSBtDGR4NsTXg7LXs35PX368m+WeiiFBZmA8S bcDllCUuDe49HNqX/CVOYuxksC+OTdIEUnuPwnE1Ahi+mVOxmkqFyzdapfAn3BKd tywqmzrZzJ5tTo2Nepgl5q7bWnXvpZ7vpW6xHbRMWojZuYtn7D2UAgfLordI57ih xI9cesrc7omysPjWB6pk+7sDX1t25f9q1yxyLnlkUK0CobkrGcA2IQTNVUIieEI/ ZBU28znNsyCMH+WIFL8xkCpSzNUn5wduwvtpTUIkPUEQ4jn3tkXR1Vr2VKgLRPgp jSyRtEnS3Wg0l8nmB+QK7u7ljZE6yd3XbjfQj0D+D0ZJVuxtzeKHUKK+uOwQK+Tb JMtVlVXwMF4dHEG4uvADvklXdI4+vhEKtxVEoTxhgK5rO0ttFReQ27u47e2twBGp I+n2Jdp6tuRftcqe+hwXo7laj/ShRjE7I0rksejqCLEKv3EmuquNAEFtcU0ioDO6 bV6Clkk+26QcwkP8PMgdpyiEsW5zvTUZBc0aSW+N39/8PzbJ+bXYmrZZIw3De/eA gXj99B/CYR/QXg8AlY/P =cwLz -----END PGP SIGNATURE----- --=-=-=--