Re: [PATCH] emacs: show: lazy part bugfix

Subject: Re: [PATCH] emacs: show: lazy part bugfix

Date: Sun, 01 Sep 2013 09:18:35 -0700

To: Mark Walters, notmuch@notmuchmail.org, amdragon@MIT.EDU

Cc:

From: Jameson Graef Rollins


On Fri, Aug 23 2013, Mark Walters <markwalters1009@gmail.com> wrote:
> The lazy part handling had a subtle bug. Notmuch stores the part
> information as a text property with the displayed part so attachment
> handling (saving viewing etc work).
>
> Now, some mime parts have subparts and to avoid overwriting the
> sub-part data notmuch checks and if part data is already recorded it
> does not overwrite it.
>
> Now with lazy part handling this could fail: there is already part
> data stored. In the common case it works as the part type information
> was stored when the lazy-part button was inserted. However, this fails
> if the lazy part has sub-parts: notmuch had no idea these existed
> until the lazy part insertion.
>
> We fix this by removing any existing part-information from the
> relevant region before doing the lazy insertion.
> ---
> This was shown up by Istvan's patch in id:m3r4dtgz9k.fsf@zsu.kismala.com and 
> the bug found by Jamie in id:87fvu4fl25.fsf@servo.finestructure.net
>
> I think this is essentially the right patch: I am not certain about
> the +1 in the removing the property. It seems to be needed but maybe
> something with front/back sticky would be better.
>
> Also this definitely needs more testing before going into master: this
> code is definitely fragile.

Hey, Mark.  I have just tested this patch and it seems to be fixing the
issue I was seeing.  With Istvan's patch the once hidden parts are now
exposed with buttons, and with this patch the buttons now work as
expected.

I can't really speak to the correctness of this patch, but it does seem
to be working, and doesn't seem to have any adverse side effects that I
can see.

As a reminder, this patch and Istvan's do fix an actual bug in notmuch,
so both should be pushed unless anyone see's anything wrong with either
of them.

Thanks so much for the fixes, guys.

jamie.
part-000.sig (application/pgp-signature)

Thread: