8c/53e1fe69ec2f0a8f5f2cf1dc76b123c75f6f25

   1 Return-Path: <dme@dme.org>\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 068FD431FBC\r
   6         for <notmuch@notmuchmail.org>; Tue, 24 Jan 2012 08:46:53 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\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 APoJSgkbLw8Q for <notmuch@notmuchmail.org>;\r
  16         Tue, 24 Jan 2012 08:46:52 -0800 (PST)\r
  17 Received: from mail-wi0-f181.google.com (mail-wi0-f181.google.com\r
  18         [209.85.212.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id EE280431FB6\r
  21         for <notmuch@notmuchmail.org>; Tue, 24 Jan 2012 08:46:51 -0800 (PST)\r
  22 Received: by wibhi8 with SMTP id hi8so1726794wib.26\r
  23         for <notmuch@notmuchmail.org>; Tue, 24 Jan 2012 08:46:50 -0800 (PST)\r
  24 Received: by 10.180.95.105 with SMTP id dj9mr21920739wib.18.1327423610756;\r
  25         Tue, 24 Jan 2012 08:46:50 -0800 (PST)\r
  26 Received: from hotblack-desiato.hh.sledj.net\r
  27         (host81-149-164-25.in-addr.btopenworld.com. [81.149.164.25])\r
  28         by mx.google.com with ESMTPS id df2sm19684042wib.4.2012.01.24.08.46.48\r
  29         (version=TLSv1/SSLv3 cipher=OTHER);\r
  30         Tue, 24 Jan 2012 08:46:49 -0800 (PST)\r
  31 Received: by hotblack-desiato.hh.sledj.net (Postfix, from userid 30000)\r
  32         id B96949FD47; Tue, 24 Jan 2012 16:46:47 +0000 (GMT)\r
  33 To: Austin Clements <amdragon@MIT.EDU>\r
  34 Subject: Re: [PATCH 1/2] emacs: Use text properties rather than overlays in\r
  35         `notmuch-show-mode'.\r
  36 In-Reply-To: <20120124161511.GC16740@mit.edu>\r
  37 References: <1327405007-4026-1-git-send-email-dme@dme.org>\r
  38         <1327405007-4026-2-git-send-email-dme@dme.org>\r
  39         <20120124161511.GC16740@mit.edu>\r
  40 User-Agent: Notmuch/0.11+106~g382b2a0 (http://notmuchmail.org) Emacs/24.0.92.1\r
  41         (x86_64-pc-linux-gnu)\r
  42 From: David Edmondson <dme@dme.org>\r
  43 Date: Tue, 24 Jan 2012 16:46:44 +0000\r
  44 Message-ID: <cunlioxoxor.fsf@hotblack-desiato.hh.sledj.net>\r
  45 MIME-Version: 1.0\r
  46 Content-Type: multipart/signed; boundary="=-=-=";\r
  47         micalg=pgp-sha1; protocol="application/pgp-signature"\r
  48 X-Gm-Message-State:\r
  49  ALoCoQkKRoBBpdT1pFXY/+pHpOgi0LDfTPERHEgf3KW0og6im0uVk1pESwki0qACJTOkrZ2UNxII\r
  50 Cc: notmuch@notmuchmail.org\r
  51 X-BeenThere: notmuch@notmuchmail.org\r
  52 X-Mailman-Version: 2.1.13\r
  53 Precedence: list\r
  54 List-Id: "Use and development of the notmuch mail system."\r
  55         <notmuch.notmuchmail.org>\r
  56 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  57         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  58 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  59 List-Post: <mailto:notmuch@notmuchmail.org>\r
  60 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  61 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  62         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  63 X-List-Received-Date: Tue, 24 Jan 2012 16:46:53 -0000\r
  64 \r
  65 --=-=-=\r
  66 Content-Type: text/plain\r
  67 Content-Transfer-Encoding: quoted-printable\r
  68 \r
  69 On Tue, 24 Jan 2012 11:15:11 -0500, Austin Clements <amdragon@MIT.EDU> wrot=\r
  70 e:\r
  71 > LGTM other than one very minor nit, though I pretty reliably make\r
  72 > fence post errors when dealing with text properties, so perhaps\r
  73 > somebody else should check that.\r
  74 >=20\r
  75 > Quoth David Edmondson on Jan 24 at 11:36 am:\r
  76 > > Except for where invisibility is involved, replace the use of overlays\r
  77 > > in `notmuch-show-mode' with text properties, which are more efficient\r
  78 > > and can be merged together more effectively.\r
  79 > > ---\r
  80 > >  emacs/notmuch-show.el |   66 +++++++++++++++++++++++++++++++----------=\r
  81 -------\r
  82 > >  emacs/notmuch-wash.el |    8 ++++-\r
  83 > >  2 files changed, 48 insertions(+), 26 deletions(-)\r
  84 > >=20\r
  85 > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el\r
  86 > > index e6a5b31..d6a0ac0 100644\r
  87 > > --- a/emacs/notmuch-show.el\r
  88 > > +++ b/emacs/notmuch-show.el\r
  89 > > @@ -258,10 +258,10 @@ operation on the contents of the current buffer."\r
  90 > >            (t\r
  91 > >             'message-header-other))))\r
  92 > >=20=20\r
  93 > > -    (overlay-put (make-overlay (point) (re-search-forward ":"))\r
  94 > > -            'face 'message-header-name)\r
  95 > > -    (overlay-put (make-overlay (point) (re-search-forward ".*$"))\r
  96 > > -            'face face)))\r
  97 > > +    (put-text-property (point) (re-search-forward ":")\r
  98 > > +                  'face 'message-header-name)\r
  99 > > +    (put-text-property (point) (re-search-forward ".*$")\r
 100 > > +                  'face face)))\r
 101 >=20\r
 102 > Does this need rear-nonsticky or are the headers already formed enough\r
 103 > by this point that it isn't necessary?\r
 104 \r
 105 It isn't necessary here.=20\r
 106 \r
 107 > >  (defun notmuch-show-colour-headers ()\r
 108 > >    "Apply some colouring to the current headers."\r
 109 > > @@ -278,12 +278,11 @@ operation on the contents of the current buffer."\r
 110 > >    "Update the displayed tags of the current message."\r
 111 > >    (save-excursion\r
 112 > >      (goto-char (notmuch-show-message-top))\r
 113 > > -    (if (re-search-forward "(\\([^()]*\\))$" (line-end-position) t)\r
 114 > > -   (let ((inhibit-read-only t))\r
 115 > > -     (replace-match (concat "("\r
 116 > > -                            (propertize (mapconcat 'identity tags " ")\r
 117 > > -                                        'face 'notmuch-tag-face)\r
 118 > > -                            ")"))))))\r
 119 > > +    (when (re-search-forward "(\\([^()]*\\))$" (line-end-position) t)\r
 120 > > +      (let ((inhibit-read-only t))\r
 121 > > +   (replace-match (propertize (mapconcat 'identity tags " ")\r
 122 > > +                              'face '(notmuch-tag-face notmuch-message-summary-face))\r
 123 > > +                  nil nil nil 1)))))\r
 124 > >=20=20\r
 125 > >  (defun notmuch-show-clean-address (address)\r
 126 > >    "Try to clean a single email ADDRESS for display.  Return\r
 127 > > @@ -310,15 +309,26 @@ unchanged ADDRESS if parsing fails."\r
 128 > >    "Insert a notmuch style headerline based on HEADERS for a\r
 129 > >  message at DEPTH in the current thread."\r
 130 > >    (let ((start (point)))\r
 131 > > -    (insert (notmuch-show-spaces-n (* notmuch-show-indent-messages-wid=\r
 132 th depth))\r
 133 > > -       (notmuch-show-clean-address (plist-get headers :From))\r
 134 > > -       " ("\r
 135 > > -       date\r
 136 > > -       ") ("\r
 137 > > -       (propertize (mapconcat 'identity tags " ")\r
 138 > > -                   'face 'notmuch-tag-face)\r
 139 > > -       ")\n")\r
 140 > > -    (overlay-put (make-overlay start (point)) 'face 'notmuch-message-s=\r
 141 ummary-face)))\r
 142 > > +    (insert\r
 143 > > +     (propertize (concat (notmuch-show-clean-address (plist-get header=\r
 144 s :From))\r
 145 > > +                    " ("\r
 146 > > +                    date\r
 147 > > +                    ") (")\r
 148 > > +            'face 'notmuch-message-summary-face)\r
 149 > > +     (propertize (mapconcat 'identity tags " ")\r
 150 > > +            'face '(notmuch-tag-face notmuch-message-summary-face))\r
 151 >=20\r
 152 > I feel like I should know this, but what's the effect of giving a list\r
 153 > as the face property?  I assume it merges them in the same way it\r
 154 > merges, say, overlay and text property faces, but the documentation\r
 155 > only says that you can give a list, not what it means.\r
 156 \r
 157 It's as though they are applied in reverse order, so attributes of the\r
 158 faces nearer the front of the list win.\r
 159 \r
 160 > > +     (propertize ")\n"\r
 161 > > +            'face 'notmuch-message-summary-face))\r
 162 > > +\r
 163 > > +    ;; Ensure that any insertions at the start of this line (usually\r
 164 > > +    ;; just spaces for indentation purposes) inherit the face of the\r
 165 > > +    ;; rest of the line...\r
 166 > > +    (put-text-property start (1+ start)\r
 167 > > +                  'front-sticky '(face))\r
 168 > > +    ;; ...and that insertions at the end of this region do _not_\r
 169 > > +    ;; inherit the face of the rest of this line.\r
 170 > > +    (put-text-property (1- (point)) (point)\r
 171 > > +                  'rear-nonsticky '(face))))\r
 172 > >=20=20\r
 173 > >  (defun notmuch-show-insert-header (header header-value)\r
 174 > >    "Insert a single header."\r
 175 > > @@ -753,8 +763,15 @@ current buffer, if possible."\r
 176 > >  (defun notmuch-show-insert-bodypart (msg part depth)\r
 177 > >    "Insert the body part PART at depth DEPTH in the current thread."\r
 178 > >    (let ((content-type (downcase (plist-get part :content-type)))\r
 179 > > -   (nth (plist-get part :id)))\r
 180 > > -    (notmuch-show-insert-bodypart-internal msg part content-type nth d=\r
 181 epth content-type))\r
 182 > > +   (nth (plist-get part :id))\r
 183 > > +   (start (point)))\r
 184 > > +    (notmuch-show-insert-bodypart-internal msg part content-type nth d=\r
 185 epth content-type)\r
 186 > > +\r
 187 > > +    ;; Ensure that face properties applied to text in the buffer by\r
 188 > > +    ;; the part handler don't leak into the following text.\r
 189 > > +    (put-text-property start (point-max)\r
 190 > > +                  'rear-nonsticky '(face)))\r
 191 > > +\r
 192 > >    ;; Some of the body part handlers leave point somewhere up in the\r
 193 > >    ;; part, so we make sure that we're down at the end.\r
 194 > >    (goto-char (point-max))\r
 195 > > @@ -845,11 +862,12 @@ current buffer, if possible."\r
 196 > >      (setq body-end (point-marker))\r
 197 > >      (setq content-end (point-marker))\r
 198 > >=20=20\r
 199 > > -    ;; Indent according to the depth in the thread.\r
 200 > > -    (indent-rigidly content-start content-end (* notmuch-show-indent-m=\r
 201 essages-width depth))\r
 202 > > -\r
 203 > >      (setq message-end (point-max-marker))\r
 204 > >=20=20\r
 205 > > +    ;; Indent according to the depth in the thread.\r
 206 > > +    (indent-rigidly message-start message-end\r
 207 > > +                       (* notmuch-show-indent-messages-width depth))\r
 208 > > +\r
 209 >=20\r
 210 > Why swap the order of the setq and the indent-rigidly?\r
 211 \r
 212 A mistake.\r
 213 \r
 214 I spent some time trying to use `line-prefix' and `wrap-prefix'\r
 215 properties to replace the indentation, but couldn't get it to work well\r
 216 when combined with invisible overlays (the effect is as though faces\r
 217 start bleeding out from the end of the invisible region).\r
 218 \r
 219 >=20\r
 220 > >      ;; Save the extents of this message over the whole text of the\r
 221 > >      ;; message.\r
 222 > >      (put-text-property message-start message-end :notmuch-message-exte=\r
 223 nt (cons message-start message-end))\r
 224 > > diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el\r
 225 > > index 5c1e830..84428a4 100644\r
 226 > > --- a/emacs/notmuch-wash.el\r
 227 > > +++ b/emacs/notmuch-wash.el\r
 228 > > @@ -183,7 +183,9 @@ insert before the button, probably for indentation."\r
 229 > >      (let* ((cite-start (match-beginning 0))\r
 230 > >        (cite-end (match-end 0))\r
 231 > >        (cite-lines (count-lines cite-start cite-end)))\r
 232 > > -      (overlay-put (make-overlay cite-start cite-end) 'face 'message-c=\r
 233 ited-text)\r
 234 > > +      (put-text-property cite-start cite-end 'face 'message-cited-text)\r
 235 > > +      ;; Ensure that the next line doesn't inherit our face.\r
 236 > > +      (put-text-property (1- cite-end) cite-end 'rear-nonsticky '(face=\r
 237 ))\r
 238 > >        (when (> cite-lines (+ notmuch-wash-citation-lines-prefix\r
 239 > >                          notmuch-wash-citation-lines-suffix\r
 240 > >                          1))\r
 241 > > @@ -205,7 +207,9 @@ insert before the button, probably for indentation."\r
 242 > >               (sig-end-marker (make-marker)))\r
 243 > >           (set-marker sig-start-marker sig-start)\r
 244 > >           (set-marker sig-end-marker (point-max))\r
 245 > > -         (overlay-put (make-overlay sig-start-marker sig-end-marker) 'fa=\r
 246 ce 'message-cited-text)\r
 247 > > +         (put-text-property sig-start-marker sig-end-marker 'face 'messa=\r
 248 ge-cited-text)\r
 249 > > +         ;; Ensure that the next line doesn't inherit our face.\r
 250 > > +         (put-text-property (1- sig-end-marker) sig-end-marker 'rear-non=\r
 251 sticky '(face))\r
 252 > >           (notmuch-wash-region-to-button\r
 253 > >            msg sig-start-marker sig-end-marker\r
 254 > >            "signature" "\n"))))))\r
 255 \r
 256 --=-=-=\r
 257 Content-Type: application/pgp-signature\r
 258 \r
 259 -----BEGIN PGP SIGNATURE-----\r
 260 Version: GnuPG v1.4.11 (GNU/Linux)\r
 261 \r
 262 iEYEARECAAYFAk8e4HQACgkQaezQq/BJZRaoTgCfeL6mtGBcSN00F/+2t6Wq8yfC\r
 263 5doAnRLScsb0XxcQ85EB79zYUo3NqV12\r
 264 =xr9t\r
 265 -----END PGP SIGNATURE-----\r
 266 --=-=-=--\r