Date: Wed, 4 Sep 2013 12:16:28 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: Jameson Graef Rollins <jrollins@finestructure.net>
Subject: Re: [PATCH v2] emacs: show: lazy part bugfix
Message-ID: <20130904161628.GC1426@mit.edu>
References: <1377246875-7784-1-git-send-email-markwalters1009@gmail.com>
	<1378279835-28288-1-git-send-email-markwalters1009@gmail.com>
	<20130904145639.GB1426@mit.edu>
	<87hae07fhs.fsf@servo.finestructure.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <87hae07fhs.fsf@servo.finestructure.net>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

Quoth Jameson Graef Rollins on Sep 04 at  8:50 am:
> On Wed, Sep 04 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> >> 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.
> >
> > This says that things fail when a lazy part has sub-parts, but not
> > what the failure is.  What is the failure?  Can you give a specific
> > sequence of events and conditions that leads to and demonstrates the
> > failure?
> >
> > (I ask not just for commit posterity, but because I actually don't
> > know, though I may have figured it out after writing the comment
> > below.)
> 
> Hey, Austin.  Here's an example of a mail that is effected the issue:
> 
> └┬╴multipart/alternative 896783 bytes
>  ├─╴text/plain 379 bytes
>  └┬╴multipart/related 892556 bytes
>   ├─╴text/html 1236 bytes
>   └─╴image/jpeg inline [photo.JPG] 890841 bytes
> 
> The multipart/related part is initially hidden.  Without Istvan's patch,
> there would be no button at all for the image/jpeg part, even when the
> multipart/related is exposed.  With Istvan's patch the image/jpeg button
> is there, but without Mark's patch the button would actually reference
> the entire multipart/alternative part, instead of just the image/jpeg.
> If I tried to save the image/jpeg I would get the entire
> multipart/alternative mime structure in plain text.

Ah.  Thanks for the example!  After staring at the patch for long
enough, I figured it had to be something like this, but still didn't
have anything concrete.  It would be great to include this in the
commit message to show exactly what was broken and what's being fixed.

Now that I'm sure what the problem was, the code itself LGTM.