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?