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 96467429E25 for ; Sun, 20 Nov 2011 10:33:14 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.799 X-Spam-Level: X-Spam-Status: No, score=-0.799 tagged_above=-999 required=5 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7] 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 DyRSI3BIWudw for ; Sun, 20 Nov 2011 10:33:13 -0800 (PST) Received: from mail-bw0-f53.google.com (mail-bw0-f53.google.com [209.85.214.53]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id 9B6BC431FD0 for ; Sun, 20 Nov 2011 10:33:13 -0800 (PST) Received: by bkaq10 with SMTP id q10so6296828bka.26 for ; Sun, 20 Nov 2011 10:33:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=from:to:subject:in-reply-to:references:user-agent:date:message-id :mime-version:content-type; bh=HYDM6GZwDNYbBaSjuYctxsxVjBi2/hJ+AeZEsqHYta8=; b=A4b1NPLRIBXJlSevv/Y1Si1LTmqVcGpnrCybAncbWX+pwSJxT2eh5F5CprQIQV0Z+b +AvknXo4oJCghq7I9o766EniZpiosgadjUqHPe7UlCyEz8O9mk9oLVR4PFeO+co1Ej4I H+0UTNFD7T1/JX2I2SeGChF7m43SIg+KhG8o8= Received: by 10.205.121.1 with SMTP id ga1mr8810833bkc.60.1321813990804; Sun, 20 Nov 2011 10:33:10 -0800 (PST) Received: from localhost ([91.144.186.21]) by mx.google.com with ESMTPS id hy13sm5440530bkc.0.2011.11.20.10.33.09 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 20 Nov 2011 10:33:09 -0800 (PST) From: Dmitry Kurochkin To: Jameson Graef Rollins , notmuch@notmuchmail.org Subject: Re: [PATCH] Output unmodified Content-Type header value for JSON format. In-Reply-To: <87d3coxu7s.fsf@servo.finestructure.net> References: <1321659905-24367-1-git-send-email-dmitry.kurochkin@gmail.com> <87fwhkyisj.fsf@servo.finestructure.net> <87wrawq1dz.fsf@gmail.com> <87d3coxu7s.fsf@servo.finestructure.net> User-Agent: Notmuch/0.10~rc1+20~gec94ced (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) Date: Sun, 20 Nov 2011 22:32:53 +0400 Message-ID: <87r512pru2.fsf@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: Sun, 20 Nov 2011 18:33:14 -0000 Hi Jameson. On Sat, 19 Nov 2011 02:49:43 -0800, Jameson Graef Rollins wrote: > On Sat, 19 Nov 2011 06:42:00 +0400, Dmitry Kurochkin wrote: > > The parameters are there for a reason. They are part of the > > content-type and are needed to handle the body properly. If you say > > that the parameters are not needed by notmuch users, that implies that > > they are handled by notmuch. Which is just not possible. > > Hey, Dimitry. At least some of the parameters in the content-type are > actually related to the mime structure itself. A good example is: > > boundary=\"=-=-=\" > > This parameter is there to tell the MIME parser how to parse the content > that follows. It is meant for the first level parser and no more. Once > the MIME has been separated into it's constituent parts, there's no need > for any further clients to know anything about boundary string. > Yes, at least in most cases. On the other hand, if you can make notmuch show raw multipart part (you can, right?), then it seems natural that notmuch provides enough information to parse it. > I would argue that notmuch is acting as the first level parser. As far > as I can tell, most of the rest of the parameters I've seen should only > be useful to the those first-level parsers. > I do not think first-level parser is a standard term. As I understand, you mean that notmuch parses MIME recursively until the leaf non-MIME parts. Correct? I do not know what parameters you have seen. The most common example of a useful parameter for second-level parsers is a charset. I do not understand why do we try to come up with excuses for not providing useful information to users. Current assumption that all parameters that notmuch does not handle are useless is plain invalid. > As Austin mentioned, is it not possible for notmuch itself to act on the > parameter to give a properly formatted output to its clients? > Please see my answer to Austin. I explained why this is not an option in general case. As for parameters that notmuch already handles, like "boundary", I just do not understand why we should invent some artificial exceptions and decide for our users what parameters are useful or useless for them instead of implementing a simple and kind of expected approach: content-type in JSON is original Content-Type header value. It makes both the code and the format simpler. > > The fact that this change happens to fix an issue with HTML charsets for > > me is just a side effect. > > But isn't that actually a large part of the issue? If this patch fixes > something that you think notmuch is doing improperly, could there not be > a test for it? > No. It just happens to be how I found the problem. The issue is: notmuch JSON format mangles Content-Type header value by throwing away useful information in some cases and adding info that was not there in others. Note that I do not mention any single parameter name here. It is a general issue, not a "charset" or "boundary" parameter issue. > However, based on your patches and as far as I can tell, this change > adds more than a boundary parameter to only crypto parts > (application/signed and application/encrypted). However, I don't think > any of the crypto functionality needs any of the extra information > provided in the extended output. If there was a test for the > functionality you think is missing, it would help bolster the case for > the additional output. > Again, the patch is not about "add boundary and other useless crypto parameters". The patch is about stop throwing away useless information. > > > > "content": [{"id": 2, > > > > - "content-type": "text/plain", > > > > "content": "This is a test signed message.\n"}, > > > > > > Without figuring out what's going on, I notice that some of the tests > > > have been modified to remove the content-type fields on a bunch of > > > parts. I think that is probably not right. > > > > > > > I tried to explain this in the preable. These parts do not have > > Content-Type in the original message. So I think it is wrong for > > notmuch JSON format to add it. > > Ah, ok, I think I understand this point. I think this is actually a > separate issue than the one the rest of the patch set is for, though. > One part of the patch is that content-type parameters are also about, > and another part is that parts without content-type shouldn't be > assigned one automatically. I personally think those should be separate > patches. > The implementation makes it not practical to separate these changes. They come as a result of the same code change. Regards, Dmitry > jamie.