1 Return-Path: <tomi.ollila@iki.fi>
\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 49067431FAF
\r
6 for <notmuch@notmuchmail.org>; Wed, 19 Dec 2012 07:52:51 -0800 (PST)
\r
7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
\r
11 X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]
\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 phsgYU+iuDm2 for <notmuch@notmuchmail.org>;
\r
16 Wed, 19 Dec 2012 07:52:49 -0800 (PST)
\r
17 Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])
\r
18 by olra.theworths.org (Postfix) with ESMTP id 64281431FAE
\r
19 for <notmuch@notmuchmail.org>; Wed, 19 Dec 2012 07:52:49 -0800 (PST)
\r
20 Received: from guru.guru-group.fi (localhost [IPv6:::1])
\r
21 by guru.guru-group.fi (Postfix) with ESMTP id F2039100094;
\r
22 Wed, 19 Dec 2012 17:52:42 +0200 (EET)
\r
23 From: Tomi Ollila <tomi.ollila@iki.fi>
\r
24 To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
\r
25 Subject: Re: [DRAFT PATCH] emacs: Eliminate buffer invisibility specs from
\r
27 In-Reply-To: <1355812810-32747-1-git-send-email-amdragon@mit.edu>
\r
28 References: <1355812810-32747-1-git-send-email-amdragon@mit.edu>
\r
29 User-Agent: Notmuch/0.14+211~g71b47e9 (http://notmuchmail.org) Emacs/24.2.1
\r
30 (x86_64-unknown-linux-gnu)
\r
31 X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL
\r
32 $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F
\r
33 !)g;OY^,BjTbr)Np:%c_o'jj,Z
\r
34 Date: Wed, 19 Dec 2012 17:52:42 +0200
\r
35 Message-ID: <m2vcby2g51.fsf@guru.guru-group.fi>
\r
37 Content-Type: text/plain
\r
38 X-BeenThere: notmuch@notmuchmail.org
\r
39 X-Mailman-Version: 2.1.13
\r
41 List-Id: "Use and development of the notmuch mail system."
\r
42 <notmuch.notmuchmail.org>
\r
43 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,
\r
44 <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
\r
45 List-Archive: <http://notmuchmail.org/pipermail/notmuch>
\r
46 List-Post: <mailto:notmuch@notmuchmail.org>
\r
47 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
\r
48 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,
\r
49 <mailto:notmuch-request@notmuchmail.org?subject=subscribe>
\r
50 X-List-Received-Date: Wed, 19 Dec 2012 15:52:51 -0000
\r
52 On Tue, Dec 18 2012, Austin Clements <amdragon@MIT.EDU> wrote:
\r
54 > Previously, all visibility in show buffers for headers, message
\r
55 > bodies, and washed text was specified by generating one or more
\r
56 > symbols for each region and creating overlays with their 'invisible
\r
57 > property set to carefully crafted combinations of these symbols.
\r
58 > Visibility was controlled not by modifying the overlays directly, but
\r
59 > by adding and removing the generated symbols from a gigantic buffer
\r
62 > This has myriad negative consequences. It's slow because Emacs'
\r
63 > display engine has to traverse the buffer invisibility list for every
\r
64 > overlay and, since every overlay has its own symbol, this makes
\r
65 > rendering O(N^2) in the number of overlays. It composes poorly
\r
66 > because symbol-type 'invisible properties are taken from the highest
\r
67 > priority overlay over a given character (which is often ambiguous!),
\r
68 > rather than being gathered from all overlays over a character. As a
\r
69 > result, we have to include symbols related to message hiding in the
\r
70 > wash code lest the wash overlays un-hide parts of hidden messages. It
\r
71 > also requires various workarounds for isearch to properly open
\r
72 > overlays, to set up buffer-invisibility-spec for
\r
73 > remove-from-invisibility-spec to work right, and to explicitly refresh
\r
74 > the display after updating the buffer invisibility spec.
\r
76 > None of this is necessary.
\r
78 > This patch converts show and wash to use simple boolean 'invisible
\r
79 > properties and to not use the buffer invisibility spec. Rather than
\r
80 > adding and removing generated symbols from the invisibility spec, the
\r
81 > code now directly toggles the 'invisible property of the appropriate
\r
82 > overlay. This speeds up rendering because the display engine only has
\r
83 > to check the boolean values of the overlays over a character. It
\r
84 > composes nicely because text will be invisible if *any* overlay over
\r
85 > it has 'invisible t, which means we can overlap invisibility overlays
\r
86 > with abandon. We no longer need any of the workarounds mentioned
\r
87 > above. And it fixes a minor bug for free: now, when isearch opens a
\r
88 > washed region, the button text will update to say "Click/Enter to
\r
89 > hide" rather than remaining unchanged.
\r
92 > Mark's part toggling code got me thinking about how needlessly
\r
93 > complicated our other show-mode invisibility code was. This patch is
\r
94 > a shot at fixing that. It will require a bit of reworking after part
\r
95 > toggling goes in (owning to the aforementioned non-composability, part
\r
96 > toggling needs to be intimately aware of wash and message hiding).
\r
97 > However, I think this patch should wait until after the release
\r
98 > freeze; this code is fragile (though much less so after this patch),
\r
99 > so I'd rather it soak for a release than cause user-visible bugs for
\r
100 > no user-visible gain.
\r
102 What is the current thought of getting this (and series of
\r
103 id:1355858880-16329-1-git-send-email-markwalters1009@gmail.com )
\r
104 pushed ? The patches doesn't look bad to me and these seems to
\r
111 > emacs/notmuch-show.el | 42 +++------------------
\r
112 > emacs/notmuch-wash.el | 97 ++++++-------------------------------------------
\r
113 > 2 files changed, 17 insertions(+), 122 deletions(-)
\r
115 > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
\r
116 > index 7d9f8a9..4bdd5af 100644
\r
117 > --- a/emacs/notmuch-show.el
\r
118 > +++ b/emacs/notmuch-show.el
\r
119 > @@ -872,27 +872,8 @@ message at DEPTH in the current thread."
\r
120 > message-start message-end
\r
121 > content-start content-end
\r
122 > headers-start headers-end
\r
123 > - body-start body-end
\r
124 > - (headers-invis-spec (notmuch-show-make-symbol "header"))
\r
125 > - (message-invis-spec (notmuch-show-make-symbol "message"))
\r
126 > (bare-subject (notmuch-show-strip-re (plist-get headers :Subject))))
\r
128 > - ;; Set `buffer-invisibility-spec' to `nil' (a list), otherwise
\r
129 > - ;; removing items from `buffer-invisibility-spec' (which is what
\r
130 > - ;; `notmuch-show-headers-visible' and
\r
131 > - ;; `notmuch-show-message-visible' do) is a no-op and has no
\r
132 > - ;; effect. This caused threads with only matching messages to have
\r
133 > - ;; those messages hidden initially because
\r
134 > - ;; `buffer-invisibility-spec' stayed `t'.
\r
136 > - ;; This needs to be set here (rather than just above the call to
\r
137 > - ;; `notmuch-show-headers-visible') because some of the part
\r
138 > - ;; rendering or body washing functions
\r
139 > - ;; (e.g. `notmuch-wash-text/plain-citations') manipulate
\r
140 > - ;; `buffer-invisibility-spec').
\r
141 > - (when (eq buffer-invisibility-spec t)
\r
142 > - (setq buffer-invisibility-spec nil))
\r
144 > (setq message-start (point-marker))
\r
146 > (notmuch-show-insert-headerline headers
\r
147 > @@ -904,9 +885,6 @@ message at DEPTH in the current thread."
\r
149 > (setq content-start (point-marker))
\r
151 > - (plist-put msg :headers-invis-spec headers-invis-spec)
\r
152 > - (plist-put msg :message-invis-spec message-invis-spec)
\r
154 > ;; Set `headers-start' to point after the 'Subject:' header to be
\r
155 > ;; compatible with the existing implementation. This just sets it
\r
156 > ;; to after the first header.
\r
157 > @@ -924,7 +902,6 @@ message at DEPTH in the current thread."
\r
159 > (setq notmuch-show-previous-subject bare-subject)
\r
161 > - (setq body-start (point-marker))
\r
162 > ;; A blank line between the headers and the body.
\r
164 > (notmuch-show-insert-body msg (plist-get msg :body)
\r
165 > @@ -932,7 +909,6 @@ message at DEPTH in the current thread."
\r
166 > ;; Ensure that the body ends with a newline.
\r
169 > - (setq body-end (point-marker))
\r
170 > (setq content-end (point-marker))
\r
172 > ;; Indent according to the depth in the thread.
\r
173 > @@ -945,11 +921,9 @@ message at DEPTH in the current thread."
\r
175 > (put-text-property message-start message-end :notmuch-message-extent (cons message-start message-end))
\r
177 > - (let ((headers-overlay (make-overlay headers-start headers-end))
\r
178 > - (invis-specs (list headers-invis-spec message-invis-spec)))
\r
179 > - (overlay-put headers-overlay 'invisible invis-specs)
\r
180 > - (overlay-put headers-overlay 'priority 10))
\r
181 > - (overlay-put (make-overlay body-start body-end) 'invisible message-invis-spec)
\r
182 > + ;; Create overlays used to control visibility
\r
183 > + (plist-put msg :headers-overlay (make-overlay headers-start headers-end))
\r
184 > + (plist-put msg :message-overlay (make-overlay headers-start content-end))
\r
186 > (plist-put msg :depth depth)
\r
188 > @@ -1349,18 +1323,12 @@ effects."
\r
189 > ;; Functions relating to the visibility of messages and their
\r
192 > -(defun notmuch-show-element-visible (props visible-p spec-property)
\r
193 > - (let ((spec (plist-get props spec-property)))
\r
195 > - (remove-from-invisibility-spec spec)
\r
196 > - (add-to-invisibility-spec spec))))
\r
198 > (defun notmuch-show-message-visible (props visible-p)
\r
199 > - (notmuch-show-element-visible props visible-p :message-invis-spec)
\r
200 > + (overlay-put (plist-get props :message-overlay) 'invisible (not visible-p))
\r
201 > (notmuch-show-set-prop :message-visible visible-p props))
\r
203 > (defun notmuch-show-headers-visible (props visible-p)
\r
204 > - (notmuch-show-element-visible props visible-p :headers-invis-spec)
\r
205 > + (overlay-put (plist-get props :headers-overlay) 'invisible (not visible-p))
\r
206 > (notmuch-show-set-prop :headers-visible visible-p props))
\r
208 > ;; Functions for setting and getting attributes of the current
\r
209 > diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
\r
210 > index 7d003a2..d6db4fa 100644
\r
211 > --- a/emacs/notmuch-wash.el
\r
212 > +++ b/emacs/notmuch-wash.el
\r
213 > @@ -96,10 +96,10 @@ this many characters or at the window width (whichever one is
\r
216 > (defun notmuch-wash-toggle-invisible-action (cite-button)
\r
217 > - (let ((invis-spec (button-get cite-button 'invisibility-spec)))
\r
218 > - (if (invisible-p invis-spec)
\r
219 > - (remove-from-invisibility-spec invis-spec)
\r
220 > - (add-to-invisibility-spec invis-spec)))
\r
221 > + ;; Toggle overlay visibility
\r
222 > + (let ((overlay (button-get cite-button 'overlay)))
\r
223 > + (overlay-put overlay 'invisible (not (overlay-get overlay 'invisible))))
\r
224 > + ;; Update button text
\r
225 > (let* ((new-start (button-start cite-button))
\r
226 > (overlay (button-get cite-button 'overlay))
\r
227 > (button-label (notmuch-wash-button-label overlay))
\r
228 > @@ -110,9 +110,7 @@ lower).")
\r
229 > (let ((old-end (button-end cite-button)))
\r
230 > (move-overlay cite-button new-start (point))
\r
231 > (delete-region (point) old-end))
\r
232 > - (goto-char (min old-point (1- (button-end cite-button)))))
\r
233 > - (force-window-update)
\r
235 > + (goto-char (min old-point (1- (button-end cite-button))))))
\r
237 > (define-button-type 'notmuch-wash-button-invisibility-toggle-type
\r
238 > 'action 'notmuch-wash-toggle-invisible-action
\r
239 > @@ -132,8 +130,8 @@ lower).")
\r
240 > :supertype 'notmuch-wash-button-invisibility-toggle-type)
\r
242 > (defun notmuch-wash-region-isearch-show (overlay)
\r
243 > - (dolist (invis-spec (overlay-get overlay 'invisible))
\r
244 > - (remove-from-invisibility-spec invis-spec)))
\r
245 > + (notmuch-wash-toggle-invisible-action
\r
246 > + (overlay-get overlay 'notmuch-wash-button)))
\r
248 > (defun notmuch-wash-button-label (overlay)
\r
249 > (let* ((type (overlay-get overlay 'type))
\r
250 > @@ -158,14 +156,10 @@ that PREFIX should not include a newline."
\r
251 > ;; since the newly created symbol has no plist.
\r
253 > (let ((overlay (make-overlay beg end))
\r
254 > - (message-invis-spec (plist-get msg :message-invis-spec))
\r
255 > - (invis-spec (make-symbol (concat "notmuch-" type "-region")))
\r
256 > (button-type (intern-soft (concat "notmuch-wash-button-"
\r
257 > type "-toggle-type"))))
\r
258 > - (add-to-invisibility-spec invis-spec)
\r
259 > - (overlay-put overlay 'invisible (list invis-spec message-invis-spec))
\r
260 > + (overlay-put overlay 'invisible t)
\r
261 > (overlay-put overlay 'isearch-open-invisible #'notmuch-wash-region-isearch-show)
\r
262 > - (overlay-put overlay 'priority 10)
\r
263 > (overlay-put overlay 'type type)
\r
264 > (goto-char (1+ end))
\r
266 > @@ -174,10 +168,10 @@ that PREFIX should not include a newline."
\r
267 > (insert-before-markers prefix))
\r
268 > (let ((button-beg (point)))
\r
269 > (insert-before-markers (notmuch-wash-button-label overlay) "\n")
\r
270 > - (make-button button-beg (1- (point))
\r
271 > - 'invisibility-spec invis-spec
\r
272 > - 'overlay overlay
\r
273 > - :type button-type)))))
\r
274 > + (let ((button (make-button button-beg (1- (point))
\r
275 > + 'overlay overlay
\r
276 > + :type button-type)))
\r
277 > + (overlay-put overlay 'notmuch-wash-button button))))))
\r
279 > (defun notmuch-wash-excerpt-citations (msg depth)
\r
280 > "Excerpt citations and up to one signature."
\r
281 > @@ -382,71 +376,4 @@ for error."
\r
285 > -;; Temporary workaround for Emacs bug #8721
\r
286 > -;; http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8721
\r
288 > -(defun notmuch-isearch-range-invisible (beg end)
\r
289 > - "Same as `isearch-range-invisible' but with fixed Emacs bug #8721."
\r
290 > - (when (/= beg end)
\r
291 > - ;; Check that invisibility runs up to END.
\r
292 > - (save-excursion
\r
293 > - (goto-char beg)
\r
294 > - (let (;; can-be-opened keeps track if we can open some overlays.
\r
295 > - (can-be-opened (eq search-invisible 'open))
\r
296 > - ;; the list of overlays that could be opened
\r
297 > - (crt-overlays nil))
\r
298 > - (when (and can-be-opened isearch-hide-immediately)
\r
299 > - (isearch-close-unnecessary-overlays beg end))
\r
300 > - ;; If the following character is currently invisible,
\r
301 > - ;; skip all characters with that same `invisible' property value.
\r
302 > - ;; Do that over and over.
\r
303 > - (while (and (< (point) end) (invisible-p (point)))
\r
304 > - (if (invisible-p (get-text-property (point) 'invisible))
\r
306 > - (goto-char (next-single-property-change (point) 'invisible
\r
308 > - ;; if text is hidden by an `invisible' text property
\r
309 > - ;; we cannot open it at all.
\r
310 > - (setq can-be-opened nil))
\r
311 > - (when can-be-opened
\r
312 > - (let ((overlays (overlays-at (point)))
\r
316 > - (while overlays
\r
317 > - (setq o (car overlays)
\r
318 > - invis-prop (overlay-get o 'invisible))
\r
319 > - (if (invisible-p invis-prop)
\r
320 > - (if (overlay-get o 'isearch-open-invisible)
\r
321 > - (setq ov-list (cons o ov-list))
\r
322 > - ;; We found one overlay that cannot be
\r
323 > - ;; opened, that means the whole chunk
\r
324 > - ;; cannot be opened.
\r
325 > - (setq can-be-opened nil)))
\r
326 > - (setq overlays (cdr overlays)))
\r
327 > - (if can-be-opened
\r
328 > - ;; It makes sense to append to the open
\r
329 > - ;; overlays list only if we know that this is
\r
331 > - (setq crt-overlays (append ov-list crt-overlays)))))
\r
332 > - (goto-char (next-overlay-change (point)))))
\r
333 > - ;; See if invisibility reaches up thru END.
\r
334 > - (if (>= (point) end)
\r
335 > - (if (and can-be-opened (consp crt-overlays))
\r
337 > - (setq isearch-opened-overlays
\r
338 > - (append isearch-opened-overlays crt-overlays))
\r
339 > - (mapc 'isearch-open-overlay-temporary crt-overlays)
\r
341 > - (setq isearch-hidden t)))))))
\r
343 > -(defadvice isearch-range-invisible (around notmuch-isearch-range-invisible-advice activate)
\r
344 > - "Call `notmuch-isearch-range-invisible' instead of the original
\r
345 > -`isearch-range-invisible' when in `notmuch-show-mode' mode."
\r
346 > - (if (eq major-mode 'notmuch-show-mode)
\r
347 > - (setq ad-return-value (notmuch-isearch-range-invisible beg end))
\r
352 > (provide 'notmuch-wash)
\r
356 > _______________________________________________
\r
357 > notmuch mailing list
\r
358 > notmuch@notmuchmail.org
\r
359 > http://notmuchmail.org/mailman/listinfo/notmuch
\r
projects / notmuch-archives.git / blob