[PATCH] notmuch/emacs: Observe the charset of encoded parts, where known.

[Dmitry Kurochkin]

Hi David.

On Thu, 12 Jan 2012 12:00:14 +0000, David Edmondson <dme at dme.org> wrote:
> On Wed, 11 Jan 2012 22:34:45 +0400, Dmitry Kurochkin <dmitry.kurochkin at gmail.com> wrote:
> > -1
> 
> Which puts me back to 0 :-(
> 
> > One particular issue with your patch is that it adds (incorrect) charset
> > parameter to plain/text parts which are converted to UTF-8 in JSON
> > output.
> 
> Patches that break things should obviously not be accepted...
> 
> > I already tried to solve the above problem using a more general approach
> > (output all content-type parameters, not just charset) [1].  There was a
> > lengthy discussion on IRC about it and it was rejected.  The consensus
> > was that we need to make some more substantial changes to JSON and raw
> > output formats to properly fix the issue (and the issue is more general
> > than what this patch fixes, other content-type parameters are useful for
> > renderers as well and should be included in the output).
> 
> ...but a useful point fix should not be blocked by theoretical future
> work.
> 

I think there is a record of useful features and fixes that were not
accepted to notmuch because of some implementation issues.  And
interested people were using them in private repos for years.  (I do not
say that it is always the right thing to do, or that it is the right
thing in this particular case.)

> > I am planning to work on a proper fix for this issue, but decided to
> > postpone it until Austin's rewrite of notmuch show is complete.
> 
> If the UTF8 text/plain part issue can be resolved, would you be happier
> to accept this as an interim fix whilst we wait for the more complete
> solution?
> 

I would like to see the following changes:

* Properly handle charset with parameters in Emacs UI.  Currently it is
  broken by your patch in one place at least:
  `notmuch-show-handlers-for' would produce incorrect results for
  content-type string with parameters.  In my patch [1] I did parse the
  charset at top level and then changed all usages of it accordingly.
  Making `notmuch-show-handlers-for' smarter about parameters may be
  sufficient, but I would like to see some more details on why adding
  parameters to content-type string does not break Emacs UI code in
  other places.

* Add charset parameter for text/html parts only.

* Use `mail-header-parse-content-type' to parse content-type instead of
  contructing the list for `mm-make-handle' manually.

* Add a proper XXX comment to notmuch-show code.

I cannot say I would be happy about this patch after these changes.  It
would be a temporary hack anyway.  But I would withdraw my -1 and let
others decide.  I would like to see Jameson and Austin input on this
one.

Regards,
  Dmitry

[1] id:"1321659905-24367-1-git-send-email-dmitry.kurochkin at gmail.com"

> Nothing in the patch (so far) should make your proposed changes any
> harder, so I'm not sure what the problem would be.