1 Return-Path: <amdragon@mit.edu>
\r
2 X-Original-To: notmuch@notmuchmail.org
\r
3 Delivered-To: notmuch@notmuchmail.org
\r
4 Received: from localhost (localhost [127.0.0.1])
\r
5 by olra.theworths.org (Postfix) with ESMTP id 60238431FAF
\r
6 for <notmuch@notmuchmail.org>; Tue, 18 Dec 2012 10:48:30 -0800 (PST)
\r
7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
\r
11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5
\r
12 tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled
\r
13 Received: from olra.theworths.org ([127.0.0.1])
\r
14 by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)
\r
15 with ESMTP id 70jvaaDkK7lu for <notmuch@notmuchmail.org>;
\r
16 Tue, 18 Dec 2012 10:48:29 -0800 (PST)
\r
17 Received: from dmz-mailsec-scanner-6.mit.edu (DMZ-MAILSEC-SCANNER-6.MIT.EDU
\r
19 by olra.theworths.org (Postfix) with ESMTP id C1373431FAE
\r
20 for <notmuch@notmuchmail.org>; Tue, 18 Dec 2012 10:48:28 -0800 (PST)
\r
21 X-AuditID: 12074423-b7fcb6d000000927-82-50d0ba7bfbb6
\r
22 Received: from mailhub-auth-2.mit.edu ( [18.7.62.36])
\r
23 by dmz-mailsec-scanner-6.mit.edu (Symantec Messaging Gateway) with SMTP
\r
24 id B4.0F.02343.B7AB0D05; Tue, 18 Dec 2012 13:48:27 -0500 (EST)
\r
25 Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])
\r
26 by mailhub-auth-2.mit.edu (8.13.8/8.9.2) with ESMTP id qBIImR8V000492;
\r
27 Tue, 18 Dec 2012 13:48:27 -0500
\r
28 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])
\r
29 (authenticated bits=0)
\r
30 (User authenticated as amdragon@ATHENA.MIT.EDU)
\r
31 by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id qBIImPoZ011088
\r
32 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);
\r
33 Tue, 18 Dec 2012 13:48:26 -0500 (EST)
\r
34 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)
\r
35 (envelope-from <amdragon@mit.edu>)
\r
36 id 1Tl2DJ-0007YS-Ea; Tue, 18 Dec 2012 13:48:25 -0500
\r
37 Date: Tue, 18 Dec 2012 13:48:25 -0500
\r
38 From: Austin Clements <amdragon@MIT.EDU>
\r
39 To: Mark Walters <markwalters1009@gmail.com>
\r
40 Subject: Re: [DRAFT PATCH] emacs: Eliminate buffer invisibility specs from
\r
42 Message-ID: <20121218184825.GM6187@mit.edu>
\r
43 References: <1355812810-32747-1-git-send-email-amdragon@mit.edu>
\r
44 <87licvqlbb.fsf@qmul.ac.uk>
\r
46 Content-Type: text/plain; charset=us-ascii
\r
47 Content-Disposition: inline
\r
48 In-Reply-To: <87licvqlbb.fsf@qmul.ac.uk>
\r
49 User-Agent: Mutt/1.5.21 (2010-09-15)
\r
50 X-Brightmail-Tracker:
\r
51 H4sIAAAAAAAAA+NgFupjleLIzCtJLcpLzFFi42IRYrdT0a3edSHA4OMKZYvVc3ksrt+cyezA
\r
52 5LFz1l12j2erbjEHMEVx2aSk5mSWpRbp2yVwZax8/J654IFtxfEdUQ2MLw26GDk5JARMJC4+
\r
53 /MwGYYtJXLi3Hsjm4hAS2Mco8eFhLyuEs4FRYu1bGOcik8TunSugypYwSjz6/QWsn0VAVWLS
\r
54 95eMIDabgIbEtv3LwWwRAR2J24cWsIPYzALSEt9+NzOB2MICURIf111hAbF5BbQlenveg80R
\r
55 EoiT2PH7DztEXFDi5MwnLBC9WhI3/r0E6uUAm7P8HwdImBNo1c1HF8BGigqoSEw5uY1tAqPQ
\r
56 LCTds5B0z0LoXsDIvIpRNiW3Sjc3MTOnODVZtzg5MS8vtUjXTC83s0QvNaV0EyM4qF2UdzD+
\r
57 Oah0iFGAg1GJh1dixYUAIdbEsuLK3EOMkhxMSqK8X7YChfiS8lMqMxKLM+KLSnNSiw8xSnAw
\r
58 K4nwnpsAlONNSaysSi3Kh0lJc7AoifNeS7npLySQnliSmp2aWpBaBJOV4eBQkuB13QnUKFiU
\r
59 mp5akZaZU4KQZuLgBBnOAzTcGaSGt7ggMbc4Mx0if4pRl6Ph5Y2njEIsefl5qVLivHwgRQIg
\r
60 RRmleXBzYMnoFaM40FvCvBogVTzARAY36RXQEiagJUF6YEtKEhFSUg2M4tXS/59Zu+vW5XKm
\r
61 zMjSTEjfYZDTdq2Q2fXrZb6f84MutAmLrLyV/PmfVblIebnLDrn5cqJVSr1bBX2v5xtaBMuk
\r
62 FOQb+WnzF7GZrlrhfpk7KLBvgQmLZY+RHHuz0se9uU8eLtn4seWLpv68TMOXMneUjX+bf2tb
\r
63 /W2DEs9TsS89MVu7/imxFGckGmoxFxUnAgCwQiidIQMAAA==
\r
64 Cc: notmuch@notmuchmail.org
\r
65 X-BeenThere: notmuch@notmuchmail.org
\r
66 X-Mailman-Version: 2.1.13
\r
68 List-Id: "Use and development of the notmuch mail system."
\r
69 <notmuch.notmuchmail.org>
\r
70 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,
\r
71 <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
\r
72 List-Archive: <http://notmuchmail.org/pipermail/notmuch>
\r
73 List-Post: <mailto:notmuch@notmuchmail.org>
\r
74 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
\r
75 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,
\r
76 <mailto:notmuch-request@notmuchmail.org?subject=subscribe>
\r
77 X-List-Received-Date: Tue, 18 Dec 2012 18:48:30 -0000
\r
79 Quoth Mark Walters on Dec 18 at 6:14 pm:
\r
80 > On Tue, 18 Dec 2012, Austin Clements <amdragon@MIT.EDU> wrote:
\r
81 > > Previously, all visibility in show buffers for headers, message
\r
82 > > bodies, and washed text was specified by generating one or more
\r
83 > > symbols for each region and creating overlays with their 'invisible
\r
84 > > property set to carefully crafted combinations of these symbols.
\r
85 > > Visibility was controlled not by modifying the overlays directly, but
\r
86 > > by adding and removing the generated symbols from a gigantic buffer
\r
87 > > invisibilty spec.
\r
89 > > This has myriad negative consequences. It's slow because Emacs'
\r
90 > > display engine has to traverse the buffer invisibility list for every
\r
91 > > overlay and, since every overlay has its own symbol, this makes
\r
92 > > rendering O(N^2) in the number of overlays. It composes poorly
\r
93 > > because symbol-type 'invisible properties are taken from the highest
\r
94 > > priority overlay over a given character (which is often ambiguous!),
\r
95 > > rather than being gathered from all overlays over a character. As a
\r
96 > > result, we have to include symbols related to message hiding in the
\r
97 > > wash code lest the wash overlays un-hide parts of hidden messages. It
\r
98 > > also requires various workarounds for isearch to properly open
\r
99 > > overlays, to set up buffer-invisibility-spec for
\r
100 > > remove-from-invisibility-spec to work right, and to explicitly refresh
\r
101 > > the display after updating the buffer invisibility spec.
\r
103 > > None of this is necessary.
\r
105 > > This patch converts show and wash to use simple boolean 'invisible
\r
106 > > properties and to not use the buffer invisibility spec. Rather than
\r
107 > > adding and removing generated symbols from the invisibility spec, the
\r
108 > > code now directly toggles the 'invisible property of the appropriate
\r
109 > > overlay. This speeds up rendering because the display engine only has
\r
110 > > to check the boolean values of the overlays over a character. It
\r
111 > > composes nicely because text will be invisible if *any* overlay over
\r
112 > > it has 'invisible t, which means we can overlap invisibility overlays
\r
113 > > with abandon. We no longer need any of the workarounds mentioned
\r
114 > > above. And it fixes a minor bug for free: now, when isearch opens a
\r
115 > > washed region, the button text will update to say "Click/Enter to
\r
116 > > hide" rather than remaining unchanged.
\r
119 > > Mark's part toggling code got me thinking about how needlessly
\r
120 > > complicated our other show-mode invisibility code was. This patch is
\r
121 > > a shot at fixing that. It will require a bit of reworking after part
\r
122 > > toggling goes in (owning to the aforementioned non-composability, part
\r
123 > > toggling needs to be intimately aware of wash and message hiding).
\r
124 > > However, I think this patch should wait until after the release
\r
125 > > freeze; this code is fragile (though much less so after this patch),
\r
126 > > so I'd rather it soak for a release than cause user-visible bugs for
\r
127 > > no user-visible gain.
\r
129 > > emacs/notmuch-show.el | 42 +++------------------
\r
130 > > emacs/notmuch-wash.el | 97 ++++++-------------------------------------------
\r
131 > > 2 files changed, 17 insertions(+), 122 deletions(-)
\r
133 > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
\r
134 > > index 7d9f8a9..4bdd5af 100644
\r
135 > > --- a/emacs/notmuch-show.el
\r
136 > > +++ b/emacs/notmuch-show.el
\r
137 > > @@ -872,27 +872,8 @@ message at DEPTH in the current thread."
\r
138 > > message-start message-end
\r
139 > > content-start content-end
\r
140 > > headers-start headers-end
\r
141 > > - body-start body-end
\r
142 > > - (headers-invis-spec (notmuch-show-make-symbol "header"))
\r
143 > > - (message-invis-spec (notmuch-show-make-symbol "message"))
\r
144 > > (bare-subject (notmuch-show-strip-re (plist-get headers :Subject))))
\r
146 > > - ;; Set `buffer-invisibility-spec' to `nil' (a list), otherwise
\r
147 > > - ;; removing items from `buffer-invisibility-spec' (which is what
\r
148 > > - ;; `notmuch-show-headers-visible' and
\r
149 > > - ;; `notmuch-show-message-visible' do) is a no-op and has no
\r
150 > > - ;; effect. This caused threads with only matching messages to have
\r
151 > > - ;; those messages hidden initially because
\r
152 > > - ;; `buffer-invisibility-spec' stayed `t'.
\r
154 > > - ;; This needs to be set here (rather than just above the call to
\r
155 > > - ;; `notmuch-show-headers-visible') because some of the part
\r
156 > > - ;; rendering or body washing functions
\r
157 > > - ;; (e.g. `notmuch-wash-text/plain-citations') manipulate
\r
158 > > - ;; `buffer-invisibility-spec').
\r
159 > > - (when (eq buffer-invisibility-spec t)
\r
160 > > - (setq buffer-invisibility-spec nil))
\r
162 > > (setq message-start (point-marker))
\r
164 > > (notmuch-show-insert-headerline headers
\r
165 > > @@ -904,9 +885,6 @@ message at DEPTH in the current thread."
\r
167 > > (setq content-start (point-marker))
\r
169 > > - (plist-put msg :headers-invis-spec headers-invis-spec)
\r
170 > > - (plist-put msg :message-invis-spec message-invis-spec)
\r
172 > > ;; Set `headers-start' to point after the 'Subject:' header to be
\r
173 > > ;; compatible with the existing implementation. This just sets it
\r
174 > > ;; to after the first header.
\r
175 > > @@ -924,7 +902,6 @@ message at DEPTH in the current thread."
\r
177 > > (setq notmuch-show-previous-subject bare-subject)
\r
179 > > - (setq body-start (point-marker))
\r
180 > > ;; A blank line between the headers and the body.
\r
182 > > (notmuch-show-insert-body msg (plist-get msg :body)
\r
183 > > @@ -932,7 +909,6 @@ message at DEPTH in the current thread."
\r
184 > > ;; Ensure that the body ends with a newline.
\r
187 > > - (setq body-end (point-marker))
\r
188 > > (setq content-end (point-marker))
\r
190 > > ;; Indent according to the depth in the thread.
\r
191 > > @@ -945,11 +921,9 @@ message at DEPTH in the current thread."
\r
193 > > (put-text-property message-start message-end :notmuch-message-extent (cons message-start message-end))
\r
195 > > - (let ((headers-overlay (make-overlay headers-start headers-end))
\r
196 > > - (invis-specs (list headers-invis-spec message-invis-spec)))
\r
197 > > - (overlay-put headers-overlay 'invisible invis-specs)
\r
198 > > - (overlay-put headers-overlay 'priority 10))
\r
199 > > - (overlay-put (make-overlay body-start body-end) 'invisible message-invis-spec)
\r
200 > > + ;; Create overlays used to control visibility
\r
201 > > + (plist-put msg :headers-overlay (make-overlay headers-start headers-end))
\r
202 > > + (plist-put msg :message-overlay (make-overlay headers-start content-end))
\r
204 > I am puzzled by this (though it was essentially the same before):
\r
205 > http://www.gnu.org/software/emacs/manual/html_node/elisp/Other-Plists.html
\r
206 > says that "This stores value as the value of the property property in
\r
207 > the property list plist. It may modify plist destructively, or it may
\r
208 > construct a new list structure without altering the old." Does this mean
\r
209 > that this might work but it might not?
\r
211 Yeah, I know. The show code does this all over the place, so I was
\r
212 being consistent. Code like this depends on unspecified behavior, but
\r
213 it is reliable given the actual implementation of plist-put, which
\r
214 will always add new properties to the end of the plist by mutation.
\r
215 Hence the only time the setq is *actually* necessary is when
\r
216 plist-put-ing to an empty plist, and msg will never be an empty plist.
\r
218 > Otherwise, though, this looks good to me.
\r
220 > My inclination is to get this in and then do the part invisibility on
\r
221 > top. One reason is that I think we definitely want to go this route
\r
222 > soon and if we do it just after the next release we may end up with bugs
\r
223 > being reported (bugs in my invisibility stuff) that there's not much
\r
224 > point in fixing as mainline has diverged.
\r