Re: [PATCH v4 0/5] Use invisibility to toggle display of all parts including multipart

[Austin Clements <amdragon@MIT.EDU>]

Quoth Mark Walters on Dec 18 at  8:54 am:
> On Mon, 17 Dec 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> > This is version 4 of this series (previous version at
> > id:1355559338-14313-1-git-send-email-markwalters1009@gmail.com).
> >
> > The only change should be a bugfix which, for reasons I don't
> > understand, only causes a problem on emacs 24. The problem is that the
> > part invisibility code looks for a part button at the start of the
> > region. This gets confused if there is a part with no part button
> > (this is the case for the first part if it is text/plain) and the part
> > starts with a button (as can happen if the message starts with the
> > reply as in the first test in test/emacs-show).
> >
> > This checks that the button is a part button before creating the part
> > overlay.
> 
> I don't think the above is very clear so I will try to explain it more
> fully. 
> 
> The invisibility overlay for a part needs to be `linked' to the part
> header button so that the part header button can toggle the overlay
> visibility. The overlay is created and linked to this button after the
> whole part has been inserted (including any notmuch-wash stuff). 
> 
> I could have made insert-part-header return the button it made and pass
> it back up the call chain to the the create-overlays function but
> instead I chose to make create-overlays just take the button at the
> start of the part.
> 
> Now if the first part is text/plain then notmuch does not insert a
> [text/plain] button so the code checks for this case by making sure the
> part does start with a button, and if not it does not create the part
> overlay (there is no button to toggle it so no point in an overlay).
> 
> However, if the first part is text/plain and notmuch wash happens to
> make a button at the very start of the part then the create-overlays
> function did still create an overlay *and* link it to the button. This
> linking overwrote some of the things notmuch wash had attached to its
> button (eg the button :overlay property) and that caused things to
> break.
> 
> I still do not know why emacs 23 and emacs 24 behave differently, but
> regardless the change from v3 is a clear bugfix: we just make sure it is
> a notmuch-show-insert-part-header button not a notmuch-wash button
> before we do the overlay creation/linking to the button. This version
> does that by looking for a :base-label property of the button which
> insert-part-header buttons have but notmuch-wash buttons do
> not. (Obviously there are other ways this check could be done)

Now I understand.  LGTM.  Do you want to go ahead and push this or
would you rather get my wash/show cleanup in and push your reworked
version of the series?