--- /dev/null
+Return-Path: <tomi.ollila@iki.fi>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id 49067431FAF\r
+ for <notmuch@notmuchmail.org>; Wed, 19 Dec 2012 07:52:51 -0800 (PST)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: 0\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
+ autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id phsgYU+iuDm2 for <notmuch@notmuchmail.org>;\r
+ Wed, 19 Dec 2012 07:52:49 -0800 (PST)\r
+Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])\r
+ by olra.theworths.org (Postfix) with ESMTP id 64281431FAE\r
+ for <notmuch@notmuchmail.org>; Wed, 19 Dec 2012 07:52:49 -0800 (PST)\r
+Received: from guru.guru-group.fi (localhost [IPv6:::1])\r
+ by guru.guru-group.fi (Postfix) with ESMTP id F2039100094;\r
+ Wed, 19 Dec 2012 17:52:42 +0200 (EET)\r
+From: Tomi Ollila <tomi.ollila@iki.fi>\r
+To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org\r
+Subject: Re: [DRAFT PATCH] emacs: Eliminate buffer invisibility specs from\r
+ show and wash\r
+In-Reply-To: <1355812810-32747-1-git-send-email-amdragon@mit.edu>\r
+References: <1355812810-32747-1-git-send-email-amdragon@mit.edu>\r
+User-Agent: Notmuch/0.14+211~g71b47e9 (http://notmuchmail.org) Emacs/24.2.1\r
+ (x86_64-unknown-linux-gnu)\r
+X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL\r
+ $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F\r
+ !)g;OY^,BjTbr)Np:%c_o'jj,Z\r
+Date: Wed, 19 Dec 2012 17:52:42 +0200\r
+Message-ID: <m2vcby2g51.fsf@guru.guru-group.fi>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Wed, 19 Dec 2012 15:52:51 -0000\r
+\r
+On Tue, Dec 18 2012, Austin Clements <amdragon@MIT.EDU> wrote:\r
+\r
+> Previously, all visibility in show buffers for headers, message\r
+> bodies, and washed text was specified by generating one or more\r
+> symbols for each region and creating overlays with their 'invisible\r
+> property set to carefully crafted combinations of these symbols.\r
+> Visibility was controlled not by modifying the overlays directly, but\r
+> by adding and removing the generated symbols from a gigantic buffer\r
+> invisibilty spec.\r
+>\r
+> This has myriad negative consequences. It's slow because Emacs'\r
+> display engine has to traverse the buffer invisibility list for every\r
+> overlay and, since every overlay has its own symbol, this makes\r
+> rendering O(N^2) in the number of overlays. It composes poorly\r
+> because symbol-type 'invisible properties are taken from the highest\r
+> priority overlay over a given character (which is often ambiguous!),\r
+> rather than being gathered from all overlays over a character. As a\r
+> result, we have to include symbols related to message hiding in the\r
+> wash code lest the wash overlays un-hide parts of hidden messages. It\r
+> also requires various workarounds for isearch to properly open\r
+> overlays, to set up buffer-invisibility-spec for\r
+> remove-from-invisibility-spec to work right, and to explicitly refresh\r
+> the display after updating the buffer invisibility spec.\r
+>\r
+> None of this is necessary.\r
+>\r
+> This patch converts show and wash to use simple boolean 'invisible\r
+> properties and to not use the buffer invisibility spec. Rather than\r
+> adding and removing generated symbols from the invisibility spec, the\r
+> code now directly toggles the 'invisible property of the appropriate\r
+> overlay. This speeds up rendering because the display engine only has\r
+> to check the boolean values of the overlays over a character. It\r
+> composes nicely because text will be invisible if *any* overlay over\r
+> it has 'invisible t, which means we can overlap invisibility overlays\r
+> with abandon. We no longer need any of the workarounds mentioned\r
+> above. And it fixes a minor bug for free: now, when isearch opens a\r
+> washed region, the button text will update to say "Click/Enter to\r
+> hide" rather than remaining unchanged.\r
+> ---\r
+>\r
+> Mark's part toggling code got me thinking about how needlessly\r
+> complicated our other show-mode invisibility code was. This patch is\r
+> a shot at fixing that. It will require a bit of reworking after part\r
+> toggling goes in (owning to the aforementioned non-composability, part\r
+> toggling needs to be intimately aware of wash and message hiding).\r
+> However, I think this patch should wait until after the release\r
+> freeze; this code is fragile (though much less so after this patch),\r
+> so I'd rather it soak for a release than cause user-visible bugs for\r
+> no user-visible gain.\r
+\r
+What is the current thought of getting this (and series of\r
+id:1355858880-16329-1-git-send-email-markwalters1009@gmail.com )\r
+pushed ? The patches doesn't look bad to me and these seems to\r
+work very well... \r
+\r
+Tomi\r
+\r
+\r
+>\r
+> emacs/notmuch-show.el | 42 +++------------------\r
+> emacs/notmuch-wash.el | 97 ++++++-------------------------------------------\r
+> 2 files changed, 17 insertions(+), 122 deletions(-)\r
+>\r
+> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el\r
+> index 7d9f8a9..4bdd5af 100644\r
+> --- a/emacs/notmuch-show.el\r
+> +++ b/emacs/notmuch-show.el\r
+> @@ -872,27 +872,8 @@ message at DEPTH in the current thread."\r
+> message-start message-end\r
+> content-start content-end\r
+> headers-start headers-end\r
+> - body-start body-end\r
+> - (headers-invis-spec (notmuch-show-make-symbol "header"))\r
+> - (message-invis-spec (notmuch-show-make-symbol "message"))\r
+> (bare-subject (notmuch-show-strip-re (plist-get headers :Subject))))\r
+> \r
+> - ;; Set `buffer-invisibility-spec' to `nil' (a list), otherwise\r
+> - ;; removing items from `buffer-invisibility-spec' (which is what\r
+> - ;; `notmuch-show-headers-visible' and\r
+> - ;; `notmuch-show-message-visible' do) is a no-op and has no\r
+> - ;; effect. This caused threads with only matching messages to have\r
+> - ;; those messages hidden initially because\r
+> - ;; `buffer-invisibility-spec' stayed `t'.\r
+> - ;;\r
+> - ;; This needs to be set here (rather than just above the call to\r
+> - ;; `notmuch-show-headers-visible') because some of the part\r
+> - ;; rendering or body washing functions\r
+> - ;; (e.g. `notmuch-wash-text/plain-citations') manipulate\r
+> - ;; `buffer-invisibility-spec').\r
+> - (when (eq buffer-invisibility-spec t)\r
+> - (setq buffer-invisibility-spec nil))\r
+> -\r
+> (setq message-start (point-marker))\r
+> \r
+> (notmuch-show-insert-headerline headers\r
+> @@ -904,9 +885,6 @@ message at DEPTH in the current thread."\r
+> \r
+> (setq content-start (point-marker))\r
+> \r
+> - (plist-put msg :headers-invis-spec headers-invis-spec)\r
+> - (plist-put msg :message-invis-spec message-invis-spec)\r
+> -\r
+> ;; Set `headers-start' to point after the 'Subject:' header to be\r
+> ;; compatible with the existing implementation. This just sets it\r
+> ;; to after the first header.\r
+> @@ -924,7 +902,6 @@ message at DEPTH in the current thread."\r
+> \r
+> (setq notmuch-show-previous-subject bare-subject)\r
+> \r
+> - (setq body-start (point-marker))\r
+> ;; A blank line between the headers and the body.\r
+> (insert "\n")\r
+> (notmuch-show-insert-body msg (plist-get msg :body)\r
+> @@ -932,7 +909,6 @@ message at DEPTH in the current thread."\r
+> ;; Ensure that the body ends with a newline.\r
+> (unless (bolp)\r
+> (insert "\n"))\r
+> - (setq body-end (point-marker))\r
+> (setq content-end (point-marker))\r
+> \r
+> ;; Indent according to the depth in the thread.\r
+> @@ -945,11 +921,9 @@ message at DEPTH in the current thread."\r
+> ;; message.\r
+> (put-text-property message-start message-end :notmuch-message-extent (cons message-start message-end))\r
+> \r
+> - (let ((headers-overlay (make-overlay headers-start headers-end))\r
+> - (invis-specs (list headers-invis-spec message-invis-spec)))\r
+> - (overlay-put headers-overlay 'invisible invis-specs)\r
+> - (overlay-put headers-overlay 'priority 10))\r
+> - (overlay-put (make-overlay body-start body-end) 'invisible message-invis-spec)\r
+> + ;; Create overlays used to control visibility\r
+> + (plist-put msg :headers-overlay (make-overlay headers-start headers-end))\r
+> + (plist-put msg :message-overlay (make-overlay headers-start content-end))\r
+> \r
+> (plist-put msg :depth depth)\r
+> \r
+> @@ -1349,18 +1323,12 @@ effects."\r
+> ;; Functions relating to the visibility of messages and their\r
+> ;; components.\r
+> \r
+> -(defun notmuch-show-element-visible (props visible-p spec-property)\r
+> - (let ((spec (plist-get props spec-property)))\r
+> - (if visible-p\r
+> - (remove-from-invisibility-spec spec)\r
+> - (add-to-invisibility-spec spec))))\r
+> -\r
+> (defun notmuch-show-message-visible (props visible-p)\r
+> - (notmuch-show-element-visible props visible-p :message-invis-spec)\r
+> + (overlay-put (plist-get props :message-overlay) 'invisible (not visible-p))\r
+> (notmuch-show-set-prop :message-visible visible-p props))\r
+> \r
+> (defun notmuch-show-headers-visible (props visible-p)\r
+> - (notmuch-show-element-visible props visible-p :headers-invis-spec)\r
+> + (overlay-put (plist-get props :headers-overlay) 'invisible (not visible-p))\r
+> (notmuch-show-set-prop :headers-visible visible-p props))\r
+> \r
+> ;; Functions for setting and getting attributes of the current\r
+> diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el\r
+> index 7d003a2..d6db4fa 100644\r
+> --- a/emacs/notmuch-wash.el\r
+> +++ b/emacs/notmuch-wash.el\r
+> @@ -96,10 +96,10 @@ this many characters or at the window width (whichever one is\r
+> lower).")\r
+> \r
+> (defun notmuch-wash-toggle-invisible-action (cite-button)\r
+> - (let ((invis-spec (button-get cite-button 'invisibility-spec)))\r
+> - (if (invisible-p invis-spec)\r
+> - (remove-from-invisibility-spec invis-spec)\r
+> - (add-to-invisibility-spec invis-spec)))\r
+> + ;; Toggle overlay visibility\r
+> + (let ((overlay (button-get cite-button 'overlay)))\r
+> + (overlay-put overlay 'invisible (not (overlay-get overlay 'invisible))))\r
+> + ;; Update button text\r
+> (let* ((new-start (button-start cite-button))\r
+> (overlay (button-get cite-button 'overlay))\r
+> (button-label (notmuch-wash-button-label overlay))\r
+> @@ -110,9 +110,7 @@ lower).")\r
+> (let ((old-end (button-end cite-button)))\r
+> (move-overlay cite-button new-start (point))\r
+> (delete-region (point) old-end))\r
+> - (goto-char (min old-point (1- (button-end cite-button)))))\r
+> - (force-window-update)\r
+> - (redisplay t))\r
+> + (goto-char (min old-point (1- (button-end cite-button))))))\r
+> \r
+> (define-button-type 'notmuch-wash-button-invisibility-toggle-type\r
+> 'action 'notmuch-wash-toggle-invisible-action\r
+> @@ -132,8 +130,8 @@ lower).")\r
+> :supertype 'notmuch-wash-button-invisibility-toggle-type)\r
+> \r
+> (defun notmuch-wash-region-isearch-show (overlay)\r
+> - (dolist (invis-spec (overlay-get overlay 'invisible))\r
+> - (remove-from-invisibility-spec invis-spec)))\r
+> + (notmuch-wash-toggle-invisible-action\r
+> + (overlay-get overlay 'notmuch-wash-button)))\r
+> \r
+> (defun notmuch-wash-button-label (overlay)\r
+> (let* ((type (overlay-get overlay 'type))\r
+> @@ -158,14 +156,10 @@ that PREFIX should not include a newline."\r
+> ;; since the newly created symbol has no plist.\r
+> \r
+> (let ((overlay (make-overlay beg end))\r
+> - (message-invis-spec (plist-get msg :message-invis-spec))\r
+> - (invis-spec (make-symbol (concat "notmuch-" type "-region")))\r
+> (button-type (intern-soft (concat "notmuch-wash-button-"\r
+> type "-toggle-type"))))\r
+> - (add-to-invisibility-spec invis-spec)\r
+> - (overlay-put overlay 'invisible (list invis-spec message-invis-spec))\r
+> + (overlay-put overlay 'invisible t)\r
+> (overlay-put overlay 'isearch-open-invisible #'notmuch-wash-region-isearch-show)\r
+> - (overlay-put overlay 'priority 10)\r
+> (overlay-put overlay 'type type)\r
+> (goto-char (1+ end))\r
+> (save-excursion\r
+> @@ -174,10 +168,10 @@ that PREFIX should not include a newline."\r
+> (insert-before-markers prefix))\r
+> (let ((button-beg (point)))\r
+> (insert-before-markers (notmuch-wash-button-label overlay) "\n")\r
+> - (make-button button-beg (1- (point))\r
+> - 'invisibility-spec invis-spec\r
+> - 'overlay overlay\r
+> - :type button-type)))))\r
+> + (let ((button (make-button button-beg (1- (point))\r
+> + 'overlay overlay\r
+> + :type button-type)))\r
+> + (overlay-put overlay 'notmuch-wash-button button))))))\r
+> \r
+> (defun notmuch-wash-excerpt-citations (msg depth)\r
+> "Excerpt citations and up to one signature."\r
+> @@ -382,71 +376,4 @@ for error."\r
+> \r
+> ;;\r
+> \r
+> -;; Temporary workaround for Emacs bug #8721\r
+> -;; http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8721\r
+> -\r
+> -(defun notmuch-isearch-range-invisible (beg end)\r
+> - "Same as `isearch-range-invisible' but with fixed Emacs bug #8721."\r
+> - (when (/= beg end)\r
+> - ;; Check that invisibility runs up to END.\r
+> - (save-excursion\r
+> - (goto-char beg)\r
+> - (let (;; can-be-opened keeps track if we can open some overlays.\r
+> - (can-be-opened (eq search-invisible 'open))\r
+> - ;; the list of overlays that could be opened\r
+> - (crt-overlays nil))\r
+> - (when (and can-be-opened isearch-hide-immediately)\r
+> - (isearch-close-unnecessary-overlays beg end))\r
+> - ;; If the following character is currently invisible,\r
+> - ;; skip all characters with that same `invisible' property value.\r
+> - ;; Do that over and over.\r
+> - (while (and (< (point) end) (invisible-p (point)))\r
+> - (if (invisible-p (get-text-property (point) 'invisible))\r
+> - (progn\r
+> - (goto-char (next-single-property-change (point) 'invisible\r
+> - nil end))\r
+> - ;; if text is hidden by an `invisible' text property\r
+> - ;; we cannot open it at all.\r
+> - (setq can-be-opened nil))\r
+> - (when can-be-opened\r
+> - (let ((overlays (overlays-at (point)))\r
+> - ov-list\r
+> - o\r
+> - invis-prop)\r
+> - (while overlays\r
+> - (setq o (car overlays)\r
+> - invis-prop (overlay-get o 'invisible))\r
+> - (if (invisible-p invis-prop)\r
+> - (if (overlay-get o 'isearch-open-invisible)\r
+> - (setq ov-list (cons o ov-list))\r
+> - ;; We found one overlay that cannot be\r
+> - ;; opened, that means the whole chunk\r
+> - ;; cannot be opened.\r
+> - (setq can-be-opened nil)))\r
+> - (setq overlays (cdr overlays)))\r
+> - (if can-be-opened\r
+> - ;; It makes sense to append to the open\r
+> - ;; overlays list only if we know that this is\r
+> - ;; t.\r
+> - (setq crt-overlays (append ov-list crt-overlays)))))\r
+> - (goto-char (next-overlay-change (point)))))\r
+> - ;; See if invisibility reaches up thru END.\r
+> - (if (>= (point) end)\r
+> - (if (and can-be-opened (consp crt-overlays))\r
+> - (progn\r
+> - (setq isearch-opened-overlays\r
+> - (append isearch-opened-overlays crt-overlays))\r
+> - (mapc 'isearch-open-overlay-temporary crt-overlays)\r
+> - nil)\r
+> - (setq isearch-hidden t)))))))\r
+> -\r
+> -(defadvice isearch-range-invisible (around notmuch-isearch-range-invisible-advice activate)\r
+> - "Call `notmuch-isearch-range-invisible' instead of the original\r
+> -`isearch-range-invisible' when in `notmuch-show-mode' mode."\r
+> - (if (eq major-mode 'notmuch-show-mode)\r
+> - (setq ad-return-value (notmuch-isearch-range-invisible beg end))\r
+> - ad-do-it))\r
+> -\r
+> -;;\r
+> -\r
+> (provide 'notmuch-wash)\r
+> -- \r
+> 1.7.10.4\r
+>\r
+> _______________________________________________\r
+> notmuch mailing list\r
+> notmuch@notmuchmail.org\r
+> http://notmuchmail.org/mailman/listinfo/notmuch\r
projects / notmuch-archives.git / commitdiff