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 3CF23431FBC for ; Tue, 24 Jan 2012 08:15:47 -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 4UCbr0oVsgmG for ; Tue, 24 Jan 2012 08:15:46 -0800 (PST) Received: from dmz-mailsec-scanner-7.mit.edu (DMZ-MAILSEC-SCANNER-7.MIT.EDU [18.7.68.36]) by olra.theworths.org (Postfix) with ESMTP id 58336431FB6 for ; Tue, 24 Jan 2012 08:15:46 -0800 (PST) X-AuditID: 12074424-b7fae6d000000906-3f-4f1ed9315ef7 Received: from mailhub-auth-2.mit.edu ( [18.7.62.36]) by dmz-mailsec-scanner-7.mit.edu (Symantec Messaging Gateway) with SMTP id 88.4A.02310.139DE1F4; Tue, 24 Jan 2012 11:15:45 -0500 (EST) Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) by mailhub-auth-2.mit.edu (8.13.8/8.9.2) with ESMTP id q0OGFjcg008605; Tue, 24 Jan 2012 11:15:45 -0500 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.6/8.12.4) with ESMTP id q0OGFhUJ025489 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT); Tue, 24 Jan 2012 11:15:44 -0500 (EST) Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77) (envelope-from ) id 1Rpj1b-0005wE-TU; Tue, 24 Jan 2012 11:15:11 -0500 Date: Tue, 24 Jan 2012 11:15:11 -0500 From: Austin Clements To: David Edmondson Subject: Re: [PATCH 1/2] emacs: Use text properties rather than overlays in `notmuch-show-mode'. Message-ID: <20120124161511.GC16740@mit.edu> References: <1327405007-4026-1-git-send-email-dme@dme.org> <1327405007-4026-2-git-send-email-dme@dme.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1327405007-4026-2-git-send-email-dme@dme.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmpmleLIzCtJLcpLzFFi42IRYrdT0TW8Kedv8OSAlcW+O1uYLK7fnMns wOSx6/lfJo9nq24xBzBFcdmkpOZklqUW6dslcGWsu8ZZ0GZZceLIPMYGxvM6XYycHBICJhL7 Jp9nhrDFJC7cW8/WxcjFISSwj1HiZfcyKGcDo8SU71/ZIZyTTBLzvv+EyixhlFj59DIjSD+L gKrEjs93WUFsNgENiW37l4PFRQQUJf5/W8EOYjMLSEt8+93MBGILCyRJ3N4xlQXE5hXQkbg2 +QdYXEggW2L58y42iLigxMmZT1ggerUkbvx7CVTDATZn+T8OkDCngLXE/RXzwdaKCqhITDm5 jW0Co9AsJN2zkHTPQuhewMi8ilE2JbdKNzcxM6c4NVm3ODkxLy+1SNdcLzezRC81pXQTIzis XVR2MDYfUjrEKMDBqMTDe2CarL8Qa2JZcWXuIUZJDiYlUd45F+X8hfiS8lMqMxKLM+KLSnNS iw8xSnAwK4nwrmwAyvGmJFZWpRblw6SkOViUxHk1tN75CQmkJ5akZqemFqQWwWRlODiUJHgr bwA1ChalpqdWpGXmlCCkmTg4QYbzAA3vBqnhLS5IzC3OTIfIn2JUlBLnLQJJCIAkMkrz4Hph aecVozjQK8K8KSBVPMCUBdf9CmgwE9DgHEWwwSWJCCmpBsb8+Xp310p//imxqIrf+uqTc4ps EaGruNKSzs6syInZuFMq+dOaht7DE0s/XGJpfFB2Q0VxwgYvhcrPB86Lvbz2/Z2I4O9TM1c8 vbZKdcafnOzjc5yjFITfbxUSXrE1Wu91zYkgxi7mlP+x9l8/mTPPEpyyfEO/7+f9WzhPxdm8 5Dhu47D5IMMcJZbijERDLeai4kQAt3oMxxYDAAA= 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: Tue, 24 Jan 2012 16:15:47 -0000 LGTM other than one very minor nit, though I pretty reliably make fence post errors when dealing with text properties, so perhaps somebody else should check that. Quoth David Edmondson on Jan 24 at 11:36 am: > Except for where invisibility is involved, replace the use of overlays > in `notmuch-show-mode' with text properties, which are more efficient > and can be merged together more effectively. > --- > emacs/notmuch-show.el | 66 +++++++++++++++++++++++++++++++----------------- > emacs/notmuch-wash.el | 8 ++++- > 2 files changed, 48 insertions(+), 26 deletions(-) > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el > index e6a5b31..d6a0ac0 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -258,10 +258,10 @@ operation on the contents of the current buffer." > (t > 'message-header-other)))) > > - (overlay-put (make-overlay (point) (re-search-forward ":")) > - 'face 'message-header-name) > - (overlay-put (make-overlay (point) (re-search-forward ".*$")) > - 'face face))) > + (put-text-property (point) (re-search-forward ":") > + 'face 'message-header-name) > + (put-text-property (point) (re-search-forward ".*$") > + 'face face))) Does this need rear-nonsticky or are the headers already formed enough by this point that it isn't necessary? > > (defun notmuch-show-colour-headers () > "Apply some colouring to the current headers." > @@ -278,12 +278,11 @@ operation on the contents of the current buffer." > "Update the displayed tags of the current message." > (save-excursion > (goto-char (notmuch-show-message-top)) > - (if (re-search-forward "(\\([^()]*\\))$" (line-end-position) t) > - (let ((inhibit-read-only t)) > - (replace-match (concat "(" > - (propertize (mapconcat 'identity tags " ") > - 'face 'notmuch-tag-face) > - ")")))))) > + (when (re-search-forward "(\\([^()]*\\))$" (line-end-position) t) > + (let ((inhibit-read-only t)) > + (replace-match (propertize (mapconcat 'identity tags " ") > + 'face '(notmuch-tag-face notmuch-message-summary-face)) > + nil nil nil 1))))) > > (defun notmuch-show-clean-address (address) > "Try to clean a single email ADDRESS for display. Return > @@ -310,15 +309,26 @@ unchanged ADDRESS if parsing fails." > "Insert a notmuch style headerline based on HEADERS for a > message at DEPTH in the current thread." > (let ((start (point))) > - (insert (notmuch-show-spaces-n (* notmuch-show-indent-messages-width depth)) > - (notmuch-show-clean-address (plist-get headers :From)) > - " (" > - date > - ") (" > - (propertize (mapconcat 'identity tags " ") > - 'face 'notmuch-tag-face) > - ")\n") > - (overlay-put (make-overlay start (point)) 'face 'notmuch-message-summary-face))) > + (insert > + (propertize (concat (notmuch-show-clean-address (plist-get headers :From)) > + " (" > + date > + ") (") > + 'face 'notmuch-message-summary-face) > + (propertize (mapconcat 'identity tags " ") > + 'face '(notmuch-tag-face notmuch-message-summary-face)) I feel like I should know this, but what's the effect of giving a list as the face property? I assume it merges them in the same way it merges, say, overlay and text property faces, but the documentation only says that you can give a list, not what it means. > + (propertize ")\n" > + 'face 'notmuch-message-summary-face)) > + > + ;; Ensure that any insertions at the start of this line (usually > + ;; just spaces for indentation purposes) inherit the face of the > + ;; rest of the line... > + (put-text-property start (1+ start) > + 'front-sticky '(face)) > + ;; ...and that insertions at the end of this region do _not_ > + ;; inherit the face of the rest of this line. > + (put-text-property (1- (point)) (point) > + 'rear-nonsticky '(face)))) > > (defun notmuch-show-insert-header (header header-value) > "Insert a single header." > @@ -753,8 +763,15 @@ current buffer, if possible." > (defun notmuch-show-insert-bodypart (msg part depth) > "Insert the body part PART at depth DEPTH in the current thread." > (let ((content-type (downcase (plist-get part :content-type))) > - (nth (plist-get part :id))) > - (notmuch-show-insert-bodypart-internal msg part content-type nth depth content-type)) > + (nth (plist-get part :id)) > + (start (point))) > + (notmuch-show-insert-bodypart-internal msg part content-type nth depth content-type) > + > + ;; Ensure that face properties applied to text in the buffer by > + ;; the part handler don't leak into the following text. > + (put-text-property start (point-max) > + 'rear-nonsticky '(face))) > + > ;; Some of the body part handlers leave point somewhere up in the > ;; part, so we make sure that we're down at the end. > (goto-char (point-max)) > @@ -845,11 +862,12 @@ current buffer, if possible." > (setq body-end (point-marker)) > (setq content-end (point-marker)) > > - ;; Indent according to the depth in the thread. > - (indent-rigidly content-start content-end (* notmuch-show-indent-messages-width depth)) > - > (setq message-end (point-max-marker)) > > + ;; Indent according to the depth in the thread. > + (indent-rigidly message-start message-end > + (* notmuch-show-indent-messages-width depth)) > + Why swap the order of the setq and the indent-rigidly? > ;; Save the extents of this message over the whole text of the > ;; message. > (put-text-property message-start message-end :notmuch-message-extent (cons message-start message-end)) > diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el > index 5c1e830..84428a4 100644 > --- a/emacs/notmuch-wash.el > +++ b/emacs/notmuch-wash.el > @@ -183,7 +183,9 @@ insert before the button, probably for indentation." > (let* ((cite-start (match-beginning 0)) > (cite-end (match-end 0)) > (cite-lines (count-lines cite-start cite-end))) > - (overlay-put (make-overlay cite-start cite-end) 'face 'message-cited-text) > + (put-text-property cite-start cite-end 'face 'message-cited-text) > + ;; Ensure that the next line doesn't inherit our face. > + (put-text-property (1- cite-end) cite-end 'rear-nonsticky '(face)) > (when (> cite-lines (+ notmuch-wash-citation-lines-prefix > notmuch-wash-citation-lines-suffix > 1)) > @@ -205,7 +207,9 @@ insert before the button, probably for indentation." > (sig-end-marker (make-marker))) > (set-marker sig-start-marker sig-start) > (set-marker sig-end-marker (point-max)) > - (overlay-put (make-overlay sig-start-marker sig-end-marker) 'face 'message-cited-text) > + (put-text-property sig-start-marker sig-end-marker 'face 'message-cited-text) > + ;; Ensure that the next line doesn't inherit our face. > + (put-text-property (1- sig-end-marker) sig-end-marker 'rear-nonsticky '(face)) > (notmuch-wash-region-to-button > msg sig-start-marker sig-end-marker > "signature" "\n"))))))