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"))))))