[PATCH] Output unmodified Content-Type header value for JSON format.

Dmitry Kurochkin dmitry.kurochkin at gmail.com
Fri Nov 18 18:42:00 PST 2011


Hi Jamie.

On Fri, 18 Nov 2011 17:58:52 -0800, Jameson Graef Rollins <jrollins at finestructure.net> wrote:
> On Sat, 19 Nov 2011 03:45:05 +0400, Dmitry Kurochkin <dmitry.kurochkin at gmail.com> wrote:
> > Before the change, notmuch used g_mime_content_type_to_string(3)
> > function to output Content-Type header value.  Turns out it outputs
> > only "type/subtype" part and ignores all parameters.  Also, if there
> > is no Content-Type header, default "text/plain" value is used.
> 
> Hi, Dmitry.  Can you explain under what circumstances you would need the
> extra content-type parameters?

Charset is an example of a parameter which you need to render a part
correctly.

>  It just seems like a lot of extra noise
> in the output to me, but that's partially because I can't think of any
> reason why something that is receiving pre-parsed mime content would
> need it.  Maybe there's a better way to handle what you're trying to get
> to.
> 

Why extra output in JSON is an issue?

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.

> I think it would help a lot if you could submit some sort of test
> modification that demonstrates the issue.  This is one of the reasons we
> keep emphasizing that it's good to first have tests in hand that
> demonstrate issues before patches that address them.
> 

The fact that this change happens to fix an issue with HTML charsets for
me is just a side effect.

The real issue is that JSON format throws away information which is
required to properly render a part.  I do not think we need to add a
dedicated test to check that JSON outputs charsets with parameters,
considering that it is already present in many other tests.

I do not think it was intended that notmuch outputs stripped
Content-Type values.  It was just a side effect of using
g_mime_content_type_to_string(3) which gone unnoticed.

> >   "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.

Regards,
  Dmitry

> jamie.


More information about the notmuch mailing list