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

[Dmitry Kurochkin]

On Thu, 12 Jan 2012 14:42:49 +0000, David Edmondson <dme at dme.org> wrote:
> On Thu, 12 Jan 2012 18:17:44 +0400, Dmitry Kurochkin <dmitry.kurochkin at gmail.com> wrote:
> > 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 agree that this has happened. I think that it's a failure of the
> project that it has become common, necessary and generally accepted.
> 
> > 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.
> 
> Your patch modifies the output of 'notmuch show' such that it included
> the full value of the content-type header, which means that it is
> necessary to parse it more carefully in emacs to discover and (as
> necessary) remove the parameters. The patch I posted doesn't do this,
> preferring to pass the charset (if any) as a supplementary parameter and
> leave the content-type as-is. This distinction means that the patch I
> posted isn't broken in the way that you describe.
> 

Sorry, I should have better look at the code.

> > * Add charset parameter for text/html parts only.
> 
> Version 2 of the patch does this.
> 
> > * Use `mail-header-parse-content-type' to parse content-type instead of
> >   contructing the list for `mm-make-handle' manually.
> 
> That's not required, see above.
> 
> > * Add a proper XXX comment to notmuch-show code.
> 
> I'm happy to do that.
> 
> > I cannot say I would be happy about this patch after these changes.
> 
> Can you say why? I agree that it is not a solution to all problems, but
> it is a workable solution to a specific problem.
> 

At the very least, because I did not really review the code, as you
probably understood from my poor comments :)

I do not have a strong preference here.  If others do, I prefer to leave
it for them to decide.

> > It would be a temporary hack anyway.
> 
> Agreed. Do you have any idea when you might be able to spend time on the
> better approach?

I hope to work on this once Austin's notmuch show rewrite is done.  It
is progressing, but I do not have any estimations.

Regards,
  Dmitry