From aa874f25bbcf170c0247b7004835f39d08b3e290 Mon Sep 17 00:00:00 2001 From: Jameson Graef Rollins Date: Sun, 4 Mar 2012 14:05:58 +1600 Subject: [PATCH] Re: [PATCH 5/5] show: Convert raw format to the new self-recursive style --- 59/430c98b92839f3db0dbfeb5e43b525be80ffbc | 160 ++++++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 59/430c98b92839f3db0dbfeb5e43b525be80ffbc diff --git a/59/430c98b92839f3db0dbfeb5e43b525be80ffbc b/59/430c98b92839f3db0dbfeb5e43b525be80ffbc new file mode 100644 index 000000000..6dc2ec997 --- /dev/null +++ b/59/430c98b92839f3db0dbfeb5e43b525be80ffbc @@ -0,0 +1,160 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +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----- +--=-=-=-- -- 2.26.2