From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 25011431FAF for ; Mon, 9 Sep 2013 06:56:54 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.7 X-Spam-Level: X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id I8h7weCTlFVp for ; Mon, 9 Sep 2013 06:56:48 -0700 (PDT) Received: from dmz-mailsec-scanner-5.mit.edu (dmz-mailsec-scanner-5.mit.edu [18.7.68.34]) by olra.theworths.org (Postfix) with ESMTP id 5B9B0431FAE for ; Mon, 9 Sep 2013 06:56:48 -0700 (PDT) X-AuditID: 12074422-b7ef78e000000935-aa-522dd39d9712 Received: from mailhub-auth-4.mit.edu ( [18.7.62.39]) by dmz-mailsec-scanner-5.mit.edu (Symantec Messaging Gateway) with SMTP id 36.31.02357.D93DD225; Mon, 9 Sep 2013 09:56:45 -0400 (EDT) Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id r89DuipH021130; Mon, 9 Sep 2013 09:56:45 -0400 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) (authenticated bits=0) (User authenticated as amdragon@ATHENA.MIT.EDU) by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id r89DufqF002473 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Mon, 9 Sep 2013 09:56:43 -0400 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80) (envelope-from ) id 1VJ1xI-0007ND-FM; Mon, 09 Sep 2013 09:56:40 -0400 Date: Mon, 9 Sep 2013 09:56:39 -0400 From: Austin Clements To: Mark Walters Subject: Re: [PATCH] emacs: show: lazy part handling bugfix Message-ID: <20130909135637.GF1426@mit.edu> References: <87txhz14z6.fsf@qmul.ac.uk> <1378510125-10245-1-git-send-email-markwalters1009@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1378510125-10245-1-git-send-email-markwalters1009@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupmleLIzCtJLcpLzFFi42IRYrdT1517WTfIYOlKU4vVc3ksrt+cyezA 5LFz1l12j2erbjEHMEVx2aSk5mSWpRbp2yVwZbz7GFXQzF/RvG0CUwPjD+4uRk4OCQETibnv drFD2GISF+6tZ+ti5OIQEtjHKDFj31MWCGcDo8S03rtsIFVCAqeYJBavkYVILGGUOPhnJVAV BweLgIrE5nfZIDVsAhoS2/YvZwSxRQR0JG4fWgC2gVlAWuLb72YmEFtYwFri3srvzCA2r4C2 xNcTs1hBxggJpEt0XQmFCAtKnJz5hAWiVUvixr+XTCAlIGOW/+MACXMKeEk0XO4E2yQKdMCU k9vYJjAKzULSPQtJ9yyE7gWMzKsYZVNyq3RzEzNzilOTdYuTE/PyUot0TfVyM0v0UlNKNzGC w9lFaQfjz4NKhxgFOBiVeHgDjukGCbEmlhVX5h5ilORgUhLlvXERKMSXlJ9SmZFYnBFfVJqT WnyIUYKDWUmEV/QwUI43JbGyKrUoHyYlzcGiJM777OnZQKB3E0tSs1NTC1KLYLIyHBxKErwX LgE1ChalpqdWpGXmlCCkmTg4QYbzAA0/AFLDW1yQmFucmQ6RP8Woy/Fn5dxPjEIsefl5qVLi vBdBigRAijJK8+DmwNLQK0ZxoLeEIUbxAFMY3KRXQEuYgJaIZIEtKUlESEk1MDJV/96gWJfJ uPdopdvkj3s2vdvxtp1T4t3J9k2VS9vMdyk8kPfsL9HosW55Vxv0MG9bqmbGFqMnh2dtW/D+ 1ZcUw4tTNG33RL/aluvHmGnX5zVJ/9+kN3Wzd7xlLZy74Nr82CccYmUO4roVjm9PvOeQ8jCY 82wBG8MSgZVHVFLkM319M/i+qCixFGckGmoxFxUnAgBf+9sOHgMAAA== Cc: notmuch@notmuchmail.org X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Sep 2013 13:56:54 -0000 Nice sleuthing! LGTM. One question, though. `notmuch-show-insert-bodypart' calls `notmuch-show-toggle-part-invisibility' with point at the end of the buffer and not on the button. Without this patch, properties will be nil as a result, but with this patch, they may not be. This still seems strictly more correct, but do you know why this is okay? Are the properties nil at this point regardless because the button is all in an overlay and we don't apply the :notmuch-part text property until after this call to `notmuch-show-toggle-part-invisibility'? Quoth Mark Walters on Sep 07 at 12:28 am: > The lazy part handler had a bug that it allowed the button to be > toggled to be specified. During toggling it needs to save and restore > the text-properties for the button but it actually saved the text > properties at point rather than from the button. > > In almost all cases this didn't matter as as point had the same text > properties as the button. However, it is a bug and did cause incorrect > behaviour in some cases: see id:87txhz14z6.fsf@qmul.ac.uk for details. > --- > This is exactly the same as the patch in the parent except it has a > commit message. > > Best wishes > > Mark > > > emacs/notmuch-show.el | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el > index 20844f0..0267574 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -503,7 +503,7 @@ message at DEPTH in the current thread." > (new-start (button-start button)) > (button-label (button-get button :base-label)) > (old-point (point)) > - (properties (text-properties-at (point))) > + (properties (text-properties-at (button-start button))) > (inhibit-read-only t)) > ;; Toggle the button itself. > (button-put button :notmuch-part-hidden (not show))