From: Austin Clements Date: Tue, 18 Dec 2012 06:40:10 +0000 (+1900) Subject: [DRAFT PATCH] emacs: Eliminate buffer invisibility specs from show and wash X-Git-Url: http://git.tremily.us/gitweb.cgi?a=commitdiff_plain;h=7cc5692d5725518481d75d04cd0637b66a304d19;p=notmuch-archives.git [DRAFT PATCH] emacs: Eliminate buffer invisibility specs from show and wash --- diff --git a/98/496a95da1379fc3b778418e2041d51166cf7c1 b/98/496a95da1379fc3b778418e2041d51166cf7c1 new file mode 100644 index 000000000..2f7fc56b2 --- /dev/null +++ b/98/496a95da1379fc3b778418e2041d51166cf7c1 @@ -0,0 +1,364 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id A7D76431FAF + for ; Mon, 17 Dec 2012 22:40:21 -0800 (PST) +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 q3zbMtBVzMjc for ; + Mon, 17 Dec 2012 22:40:20 -0800 (PST) +Received: from dmz-mailsec-scanner-3.mit.edu (DMZ-MAILSEC-SCANNER-3.MIT.EDU + [18.9.25.14]) + by olra.theworths.org (Postfix) with ESMTP id 3B913431FAE + for ; Mon, 17 Dec 2012 22:40:20 -0800 (PST) +X-AuditID: 1209190e-b7f516d0000008e4-da-50d00fd3d80f +Received: from mailhub-auth-4.mit.edu ( [18.7.62.39]) + by dmz-mailsec-scanner-3.mit.edu (Symantec Messaging Gateway) with SMTP + id 39.E9.02276.3DF00D05; Tue, 18 Dec 2012 01:40:19 -0500 (EST) +Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) + by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id qBI6eH6N003078; + Tue, 18 Dec 2012 01:40:17 -0500 +Received: from drake.dyndns.org + (209-6-116-242.c3-0.arl-ubr1.sbo-arl.ma.cable.rcn.com + [209.6.116.242]) (authenticated bits=0) + (User authenticated as amdragon@ATHENA.MIT.EDU) + by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id qBI6eFsS029846 + (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT); + Tue, 18 Dec 2012 01:40:17 -0500 (EST) +Received: from amthrax by drake.dyndns.org with local (Exim 4.77) + (envelope-from ) + id 1Tkqqd-00006l-AL; Tue, 18 Dec 2012 01:40:15 -0500 +From: Austin Clements +To: notmuch@notmuchmail.org +Subject: [DRAFT PATCH] emacs: Eliminate buffer invisibility specs from show + and wash +Date: Tue, 18 Dec 2012 01:40:10 -0500 +Message-Id: <1355812810-32747-1-git-send-email-amdragon@mit.edu> +X-Mailer: git-send-email 1.7.10.4 +X-Brightmail-Tracker: + H4sIAAAAAAAAA+NgFlrKIsWRmVeSWpSXmKPExsUixG6nrnuZ/0KAwdezGhbXb85kdmD0eLbq + FnMAYxSXTUpqTmZZapG+XQJXxoW179gKXqdUnP/5n7GB8VlgFyMHh4SAicSa2xVdjJxAppjE + hXvr2boYuTiEBPYxSuw/vJwdwtnAKHHr/EaozCMmif07p0Fl5jJKbFz3nBWkn01AQ2Lb/uWM + ILaIgLTEzruzWUFWMAuoSfzpUgEJCwuESJy618sEYrMIqEr82vaIHaSEV8BBouuXLcQVihLd + zyawTWDkXcDIsIpRNiW3Sjc3MTOnODVZtzg5MS8vtUjXWC83s0QvNaV0EyMoBDgl+XYwfj2o + dIhRgINRiYf3R/z5ACHWxLLiytxDjJIcTEqivNV8FwKE+JLyUyozEosz4otKc1KLDzFKcDAr + ifDeXgNUzpuSWFmVWpQPk5LmYFES572SctNfSCA9sSQ1OzW1ILUIJivDwaEkwXsAZKhgUWp6 + akVaZk4JQpqJgxNkOA/Q8CUgNbzFBYm5xZnpEPlTjLocDS9vPGUUYsnLz0uVEufdCFIkAFKU + UZoHNwcWu68YxYHeEuZdA1LFA4x7uEmvgJYwAS1ZbnMGZElJIkJKqoGxMefs8us3rUU6bd5m + 7so8K/2hcnPY3ppiCZOrt+vfd7CbzudYNkes6F3pc+GL881f5ct/emZ4dPWZHCvt7M6L0Q8e + rg0P3Gn6KySJ67jyss0SZ/Ze3nvDdv6MWfOKju2LsubYGO3N9qrlX+3h3wsFCppD34TJuVy5 + zyZs99ZOZ5aN3u0DS48sU2Ipzkg01GIuKk4EAN5QPlG4AgAA +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: Tue, 18 Dec 2012 06:40:21 -0000 + +Previously, all visibility in show buffers for headers, message +bodies, and washed text was specified by generating one or more +symbols for each region and creating overlays with their 'invisible +property set to carefully crafted combinations of these symbols. +Visibility was controlled not by modifying the overlays directly, but +by adding and removing the generated symbols from a gigantic buffer +invisibilty spec. + +This has myriad negative consequences. It's slow because Emacs' +display engine has to traverse the buffer invisibility list for every +overlay and, since every overlay has its own symbol, this makes +rendering O(N^2) in the number of overlays. It composes poorly +because symbol-type 'invisible properties are taken from the highest +priority overlay over a given character (which is often ambiguous!), +rather than being gathered from all overlays over a character. As a +result, we have to include symbols related to message hiding in the +wash code lest the wash overlays un-hide parts of hidden messages. It +also requires various workarounds for isearch to properly open +overlays, to set up buffer-invisibility-spec for +remove-from-invisibility-spec to work right, and to explicitly refresh +the display after updating the buffer invisibility spec. + +None of this is necessary. + +This patch converts show and wash to use simple boolean 'invisible +properties and to not use the buffer invisibility spec. Rather than +adding and removing generated symbols from the invisibility spec, the +code now directly toggles the 'invisible property of the appropriate +overlay. This speeds up rendering because the display engine only has +to check the boolean values of the overlays over a character. It +composes nicely because text will be invisible if *any* overlay over +it has 'invisible t, which means we can overlap invisibility overlays +with abandon. We no longer need any of the workarounds mentioned +above. And it fixes a minor bug for free: now, when isearch opens a +washed region, the button text will update to say "Click/Enter to +hide" rather than remaining unchanged. +--- + +Mark's part toggling code got me thinking about how needlessly +complicated our other show-mode invisibility code was. This patch is +a shot at fixing that. It will require a bit of reworking after part +toggling goes in (owning to the aforementioned non-composability, part +toggling needs to be intimately aware of wash and message hiding). +However, I think this patch should wait until after the release +freeze; this code is fragile (though much less so after this patch), +so I'd rather it soak for a release than cause user-visible bugs for +no user-visible gain. + + emacs/notmuch-show.el | 42 +++------------------ + emacs/notmuch-wash.el | 97 ++++++------------------------------------------- + 2 files changed, 17 insertions(+), 122 deletions(-) + +diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el +index 7d9f8a9..4bdd5af 100644 +--- a/emacs/notmuch-show.el ++++ b/emacs/notmuch-show.el +@@ -872,27 +872,8 @@ message at DEPTH in the current thread." + message-start message-end + content-start content-end + headers-start headers-end +- body-start body-end +- (headers-invis-spec (notmuch-show-make-symbol "header")) +- (message-invis-spec (notmuch-show-make-symbol "message")) + (bare-subject (notmuch-show-strip-re (plist-get headers :Subject)))) + +- ;; Set `buffer-invisibility-spec' to `nil' (a list), otherwise +- ;; removing items from `buffer-invisibility-spec' (which is what +- ;; `notmuch-show-headers-visible' and +- ;; `notmuch-show-message-visible' do) is a no-op and has no +- ;; effect. This caused threads with only matching messages to have +- ;; those messages hidden initially because +- ;; `buffer-invisibility-spec' stayed `t'. +- ;; +- ;; This needs to be set here (rather than just above the call to +- ;; `notmuch-show-headers-visible') because some of the part +- ;; rendering or body washing functions +- ;; (e.g. `notmuch-wash-text/plain-citations') manipulate +- ;; `buffer-invisibility-spec'). +- (when (eq buffer-invisibility-spec t) +- (setq buffer-invisibility-spec nil)) +- + (setq message-start (point-marker)) + + (notmuch-show-insert-headerline headers +@@ -904,9 +885,6 @@ message at DEPTH in the current thread." + + (setq content-start (point-marker)) + +- (plist-put msg :headers-invis-spec headers-invis-spec) +- (plist-put msg :message-invis-spec message-invis-spec) +- + ;; Set `headers-start' to point after the 'Subject:' header to be + ;; compatible with the existing implementation. This just sets it + ;; to after the first header. +@@ -924,7 +902,6 @@ message at DEPTH in the current thread." + + (setq notmuch-show-previous-subject bare-subject) + +- (setq body-start (point-marker)) + ;; A blank line between the headers and the body. + (insert "\n") + (notmuch-show-insert-body msg (plist-get msg :body) +@@ -932,7 +909,6 @@ message at DEPTH in the current thread." + ;; Ensure that the body ends with a newline. + (unless (bolp) + (insert "\n")) +- (setq body-end (point-marker)) + (setq content-end (point-marker)) + + ;; Indent according to the depth in the thread. +@@ -945,11 +921,9 @@ message at DEPTH in the current thread." + ;; message. + (put-text-property message-start message-end :notmuch-message-extent (cons message-start message-end)) + +- (let ((headers-overlay (make-overlay headers-start headers-end)) +- (invis-specs (list headers-invis-spec message-invis-spec))) +- (overlay-put headers-overlay 'invisible invis-specs) +- (overlay-put headers-overlay 'priority 10)) +- (overlay-put (make-overlay body-start body-end) 'invisible message-invis-spec) ++ ;; Create overlays used to control visibility ++ (plist-put msg :headers-overlay (make-overlay headers-start headers-end)) ++ (plist-put msg :message-overlay (make-overlay headers-start content-end)) + + (plist-put msg :depth depth) + +@@ -1349,18 +1323,12 @@ effects." + ;; Functions relating to the visibility of messages and their + ;; components. + +-(defun notmuch-show-element-visible (props visible-p spec-property) +- (let ((spec (plist-get props spec-property))) +- (if visible-p +- (remove-from-invisibility-spec spec) +- (add-to-invisibility-spec spec)))) +- + (defun notmuch-show-message-visible (props visible-p) +- (notmuch-show-element-visible props visible-p :message-invis-spec) ++ (overlay-put (plist-get props :message-overlay) 'invisible (not visible-p)) + (notmuch-show-set-prop :message-visible visible-p props)) + + (defun notmuch-show-headers-visible (props visible-p) +- (notmuch-show-element-visible props visible-p :headers-invis-spec) ++ (overlay-put (plist-get props :headers-overlay) 'invisible (not visible-p)) + (notmuch-show-set-prop :headers-visible visible-p props)) + + ;; Functions for setting and getting attributes of the current +diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el +index 7d003a2..d6db4fa 100644 +--- a/emacs/notmuch-wash.el ++++ b/emacs/notmuch-wash.el +@@ -96,10 +96,10 @@ this many characters or at the window width (whichever one is + lower).") + + (defun notmuch-wash-toggle-invisible-action (cite-button) +- (let ((invis-spec (button-get cite-button 'invisibility-spec))) +- (if (invisible-p invis-spec) +- (remove-from-invisibility-spec invis-spec) +- (add-to-invisibility-spec invis-spec))) ++ ;; Toggle overlay visibility ++ (let ((overlay (button-get cite-button 'overlay))) ++ (overlay-put overlay 'invisible (not (overlay-get overlay 'invisible)))) ++ ;; Update button text + (let* ((new-start (button-start cite-button)) + (overlay (button-get cite-button 'overlay)) + (button-label (notmuch-wash-button-label overlay)) +@@ -110,9 +110,7 @@ lower).") + (let ((old-end (button-end cite-button))) + (move-overlay cite-button new-start (point)) + (delete-region (point) old-end)) +- (goto-char (min old-point (1- (button-end cite-button))))) +- (force-window-update) +- (redisplay t)) ++ (goto-char (min old-point (1- (button-end cite-button)))))) + + (define-button-type 'notmuch-wash-button-invisibility-toggle-type + 'action 'notmuch-wash-toggle-invisible-action +@@ -132,8 +130,8 @@ lower).") + :supertype 'notmuch-wash-button-invisibility-toggle-type) + + (defun notmuch-wash-region-isearch-show (overlay) +- (dolist (invis-spec (overlay-get overlay 'invisible)) +- (remove-from-invisibility-spec invis-spec))) ++ (notmuch-wash-toggle-invisible-action ++ (overlay-get overlay 'notmuch-wash-button))) + + (defun notmuch-wash-button-label (overlay) + (let* ((type (overlay-get overlay 'type)) +@@ -158,14 +156,10 @@ that PREFIX should not include a newline." + ;; since the newly created symbol has no plist. + + (let ((overlay (make-overlay beg end)) +- (message-invis-spec (plist-get msg :message-invis-spec)) +- (invis-spec (make-symbol (concat "notmuch-" type "-region"))) + (button-type (intern-soft (concat "notmuch-wash-button-" + type "-toggle-type")))) +- (add-to-invisibility-spec invis-spec) +- (overlay-put overlay 'invisible (list invis-spec message-invis-spec)) ++ (overlay-put overlay 'invisible t) + (overlay-put overlay 'isearch-open-invisible #'notmuch-wash-region-isearch-show) +- (overlay-put overlay 'priority 10) + (overlay-put overlay 'type type) + (goto-char (1+ end)) + (save-excursion +@@ -174,10 +168,10 @@ that PREFIX should not include a newline." + (insert-before-markers prefix)) + (let ((button-beg (point))) + (insert-before-markers (notmuch-wash-button-label overlay) "\n") +- (make-button button-beg (1- (point)) +- 'invisibility-spec invis-spec +- 'overlay overlay +- :type button-type))))) ++ (let ((button (make-button button-beg (1- (point)) ++ 'overlay overlay ++ :type button-type))) ++ (overlay-put overlay 'notmuch-wash-button button)))))) + + (defun notmuch-wash-excerpt-citations (msg depth) + "Excerpt citations and up to one signature." +@@ -382,71 +376,4 @@ for error." + + ;; + +-;; Temporary workaround for Emacs bug #8721 +-;; http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8721 +- +-(defun notmuch-isearch-range-invisible (beg end) +- "Same as `isearch-range-invisible' but with fixed Emacs bug #8721." +- (when (/= beg end) +- ;; Check that invisibility runs up to END. +- (save-excursion +- (goto-char beg) +- (let (;; can-be-opened keeps track if we can open some overlays. +- (can-be-opened (eq search-invisible 'open)) +- ;; the list of overlays that could be opened +- (crt-overlays nil)) +- (when (and can-be-opened isearch-hide-immediately) +- (isearch-close-unnecessary-overlays beg end)) +- ;; If the following character is currently invisible, +- ;; skip all characters with that same `invisible' property value. +- ;; Do that over and over. +- (while (and (< (point) end) (invisible-p (point))) +- (if (invisible-p (get-text-property (point) 'invisible)) +- (progn +- (goto-char (next-single-property-change (point) 'invisible +- nil end)) +- ;; if text is hidden by an `invisible' text property +- ;; we cannot open it at all. +- (setq can-be-opened nil)) +- (when can-be-opened +- (let ((overlays (overlays-at (point))) +- ov-list +- o +- invis-prop) +- (while overlays +- (setq o (car overlays) +- invis-prop (overlay-get o 'invisible)) +- (if (invisible-p invis-prop) +- (if (overlay-get o 'isearch-open-invisible) +- (setq ov-list (cons o ov-list)) +- ;; We found one overlay that cannot be +- ;; opened, that means the whole chunk +- ;; cannot be opened. +- (setq can-be-opened nil))) +- (setq overlays (cdr overlays))) +- (if can-be-opened +- ;; It makes sense to append to the open +- ;; overlays list only if we know that this is +- ;; t. +- (setq crt-overlays (append ov-list crt-overlays))))) +- (goto-char (next-overlay-change (point))))) +- ;; See if invisibility reaches up thru END. +- (if (>= (point) end) +- (if (and can-be-opened (consp crt-overlays)) +- (progn +- (setq isearch-opened-overlays +- (append isearch-opened-overlays crt-overlays)) +- (mapc 'isearch-open-overlay-temporary crt-overlays) +- nil) +- (setq isearch-hidden t))))))) +- +-(defadvice isearch-range-invisible (around notmuch-isearch-range-invisible-advice activate) +- "Call `notmuch-isearch-range-invisible' instead of the original +-`isearch-range-invisible' when in `notmuch-show-mode' mode." +- (if (eq major-mode 'notmuch-show-mode) +- (setq ad-return-value (notmuch-isearch-range-invisible beg end)) +- ad-do-it)) +- +-;; +- + (provide 'notmuch-wash) +-- +1.7.10.4 +