01/e81d00a5b7636cbfca54adfbe6ce023ab7ff2b

   1 Return-Path: <amdragon@mit.edu>\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 F1765431FCB\r
   6         for <notmuch@notmuchmail.org>; Tue, 22 Jan 2013 16:17:45 -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.574\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0.574 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7, RDNS_NONE=1.274] 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 GpaVNugkdIZb for <notmuch@notmuchmail.org>;\r
  16         Tue, 22 Jan 2013 16:17:42 -0800 (PST)\r
  17 Received: from dmz-mailsec-scanner-5.mit.edu (unknown [18.7.68.34])\r
  18         by olra.theworths.org (Postfix) with ESMTP id AD51D431FC0\r
  19         for <notmuch@notmuchmail.org>; Tue, 22 Jan 2013 16:17:42 -0800 (PST)\r
  20 X-AuditID: 12074422-b7f5c6d000000545-33-50ff2c261ad8\r
  21 Received: from mailhub-auth-1.mit.edu ( [18.9.21.35])\r
  22         by dmz-mailsec-scanner-5.mit.edu (Symantec Messaging Gateway) with SMTP\r
  23         id AD.60.01349.62C2FF05; Tue, 22 Jan 2013 19:17:42 -0500 (EST)\r
  24 Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
  25         by mailhub-auth-1.mit.edu (8.13.8/8.9.2) with ESMTP id r0N0Hfir024737; \r
  26         Tue, 22 Jan 2013 19:17:42 -0500\r
  27 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  28         (authenticated bits=0)\r
  29         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  30         by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id r0N0HdcA011846\r
  31         (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
  32         Tue, 22 Jan 2013 19:17:40 -0500 (EST)\r
  33 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
  34         (envelope-from <amdragon@mit.edu>)\r
  35         id 1Txo26-0004Ph-Qe; Tue, 22 Jan 2013 19:17:38 -0500\r
  36 From: Austin Clements <amdragon@MIT.EDU>\r
  37 To: Damien Cassou <damien.cassou@gmail.com>,\r
  38         notmuch mailing list <notmuch@notmuchmail.org>\r
  39 Subject: Re: [PATCH 3/4] emacs: possibility to customize the rendering of tags\r
  40 In-Reply-To: <1358525039-13569-4-git-send-email-damien.cassou@gmail.com>\r
  41 References: <1358525039-13569-1-git-send-email-damien.cassou@gmail.com>\r
  42         <1358525039-13569-4-git-send-email-damien.cassou@gmail.com>\r
  43 User-Agent: Notmuch/0.14+243~g18d79d1 (http://notmuchmail.org) Emacs/23.4.1\r
  44         (i486-pc-linux-gnu)\r
  45 Date: Tue, 22 Jan 2013 19:17:38 -0500\r
  46 Message-ID: <87mww0da71.fsf@awakening.csail.mit.edu>\r
  47 MIME-Version: 1.0\r
  48 Content-Type: text/plain; charset=utf-8\r
  49 Content-Transfer-Encoding: quoted-printable\r
  50 X-Brightmail-Tracker:\r
  51  H4sIAAAAAAAAA+NgFuphleLIzCtJLcpLzFFi42IR4hRV1lXT+R9g0HqHx2LX3a1MFtdvzmR2\r
  52         YPLYOesuu8ezVbeYA5iiuGxSUnMyy1KL9O0SuDImdSxlK3gfXLHn0V6WBsbrjl2MnBwSAiYS\r
  53         0+Y/Y4ewxSQu3FvP1sXIxSEksI9R4tqqvewQzgZGid2PprFAOBeZJN43/WUDaRESWMIoce57\r
  54         PYjNJqAhsW3/ckYQW0QgRWLrnQ9gNcICfhI7Ji9kBbE5BTwkWhe0MkMMamaUODxtEzNIQlQg\r
  55         XmLOhqlgd7AIqErsO/6ECcTmBbpv5ZR3LBC2oMTJmU/AbGYBdYk/8y4xQ9jaEssWvmaewCg4\r
  56         C0nZLCRls5CULWBkXsUom5JbpZubmJlTnJqsW5ycmJeXWqRrqpebWaKXmlK6iREUxOwuSjsY\r
  57         fx5UOsQowMGoxMPL+eRfgBBrYllxZe4hRkkOJiVR3mSV/wFCfEn5KZUZicUZ8UWlOanFhxgl\r
  58         OJiVRHiVNwKV86YkVlalFuXDpKQ5WJTEea+l3PQXEkhPLEnNTk0tSC2CycpwcChJ8DprAw0V\r
  59         LEpNT61Iy8wpQUgzcXCCDOcBGg5Ww1tckJhbnJkOkT/FaMyx/0n7c0aOJT+ApBBLXn5eqpQ4\r
  60         rwRIqQBIaUZpHtw0WCJ6xSgO9JwwrzpIFQ8wicHNewW0igloFe/i3yCrShIRUlINjEpmbu8P\r
  61         9W2MdY5bVVHJlKNy45TAo2tsqu/3/3/FUfP6mvCJpUrHDtoIfTgrIdr8a0/0TZZGuVnBNf3n\r
  62         0z9dcjkewP91w+muCPZZ736w8537F3X8wdq3V7k2Vt/g2brrb2H7E4m+1TwxR2xyjN20vIKO\r
  63         vboVJSG6ny/p3/2+SvWJFysc/J2P5SqxFGckGmoxFxUnAgA2xgozHwMAAA==\r
  64 X-BeenThere: notmuch@notmuchmail.org\r
  65 X-Mailman-Version: 2.1.13\r
  66 Precedence: list\r
  67 List-Id: "Use and development of the notmuch mail system."\r
  68         <notmuch.notmuchmail.org>\r
  69 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  70         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  71 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  72 List-Post: <mailto:notmuch@notmuchmail.org>\r
  73 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  74 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  75         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  76 X-List-Received-Date: Wed, 23 Jan 2013 00:17:46 -0000\r
  77 \r
  78 On Fri, 18 Jan 2013, Damien Cassou <damien.cassou@gmail.com> wrote:\r
  79 > This patch extracts the rendering of tags in notmuch-show to a\r
  80 > dedicated notmuch-tagger file.\r
  81 >\r
  82 > This new file introduces a `notmuch-tagger-formats' variable that\r
  83 > associates each tag to a particular format. For example,\r
  84 >\r
  85 > (("unread"\r
  86 >   (:propertize "unread" face\r
  87 >                (:foreground "red")))\r
  88 >  ("flagged"\r
  89 >   (:propertize "flagged" display\r
  90 >                (image :type svg :file "~/notmuch/emacs/resources/star.svg=\r
  91 " :ascent center :mask heuristic))))\r
  92 >\r
  93 > associates a red font to the "unread" tag and a star picture to\r
  94 > the "flagged" tag.\r
  95 >\r
  96 > In the future, I would like to use the Customization interface of\r
  97 > Emacs to edit this variable and also to provide high-lever\r
  98 \r
  99 s/lever/level/\r
 100 \r
 101 > functions to manipulate it such\r
 102 > that (notmuch-tagger-propertize "unread" :foreground "red")\r
 103 > and (notmuch-tagger-picture "flagged" "star.svg").\r
 104 >\r
 105 > `mode-line-format' templates are used to represent the format for\r
 106 > each tag. This is a concize format that can also be used in\r
 107 \r
 108 "concise"?\r
 109 \r
 110 > `header-line-format' if later desired.\r
 111 >\r
 112 > Signed-off-by: Damien Cassou <damien.cassou@gmail.com>\r
 113 > ---\r
 114 >  emacs/notmuch-show.el   |    7 ++---\r
 115 >  emacs/notmuch-tagger.el |   75 +++++++++++++++++++++++++++++++++++++++++=\r
 116 ++++++\r
 117 >  emacs/notmuch.el        |    5 ++--\r
 118 >  3 files changed, 80 insertions(+), 7 deletions(-)\r
 119 >  create mode 100644 emacs/notmuch-tagger.el\r
 120 >\r
 121 > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el\r
 122 > index 1864dd1..7bf9f3c 100644\r
 123 > --- a/emacs/notmuch-show.el\r
 124 > +++ b/emacs/notmuch-show.el\r
 125 > @@ -36,6 +36,7 @@\r
 126 >  (require 'notmuch-mua)\r
 127 >  (require 'notmuch-crypto)\r
 128 >  (require 'notmuch-print)\r
 129 > +(require 'notmuch-tagger)\r
 130 >=20=20\r
 131 >  (declare-function notmuch-call-notmuch-process "notmuch" (&rest args))\r
 132 >  (declare-function notmuch-fontify-headers "notmuch" nil)\r
 133 > @@ -362,8 +363,7 @@ operation on the contents of the current buffer."\r
 134 >      (if (re-search-forward "(\\([^()]*\\))$" (line-end-position) t)\r
 135 >       (let ((inhibit-read-only t))\r
 136 >         (replace-match (concat "("\r
 137 > -                              (propertize (mapconcat 'identity tags " ")\r
 138 > -                                          'face 'notmuch-tag-face)\r
 139 > +                              (notmuch-tagger-present-tags tags)\r
 140 >                                ")"))))))\r
 141 >=20=20\r
 142 >  (defun notmuch-clean-address (address)\r
 143 > @@ -441,8 +441,7 @@ message at DEPTH in the current thread."\r
 144 >           " ("\r
 145 >           date\r
 146 >           ") ("\r
 147 > -         (propertize (mapconcat 'identity tags " ")\r
 148 > -                     'face 'notmuch-tag-face)\r
 149 > +         (notmuch-tagger-present-tags tags)\r
 150 >           ")\n")\r
 151 >      (overlay-put (make-overlay start (point)) 'face 'notmuch-message-sum=\r
 152 mary-face)))\r
 153 >=20=20\r
 154 > diff --git a/emacs/notmuch-tagger.el b/emacs/notmuch-tagger.el\r
 155 > new file mode 100644\r
 156 > index 0000000..90730f6\r
 157 > --- /dev/null\r
 158 > +++ b/emacs/notmuch-tagger.el\r
 159 > @@ -0,0 +1,75 @@\r
 160 > +;; notmuch-tagger.el --- Library to improve the way tags are displayed\r
 161 \r
 162 The verb-er naming scheme made sense when this wasn't part of notmuch,\r
 163 but it seems to needlessly (and confusingly) set it apart from standard\r
 164 notmuch functionality.  Any thoughts on including this straight in\r
 165 notmuch-tag.el (and adding your author and copyright to that file, of\r
 166 course)?\r
 167 \r
 168 > +;;\r
 169 > +;; Copyright =C2=A9 Damien Cassou\r
 170 > +;;\r
 171 > +;; This file is part of Notmuch.\r
 172 > +;;\r
 173 > +;; Notmuch is free software: you can redistribute it and/or modify it\r
 174 > +;; under the terms of the GNU General Public License as published by\r
 175 > +;; the Free Software Foundation, either version 3 of the License, or\r
 176 > +;; (at your option) any later version.\r
 177 > +;;\r
 178 > +;; Notmuch is distributed in the hope that it will be useful, but\r
 179 > +;; WITHOUT ANY WARRANTY; without even the implied warranty of\r
 180 > +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU\r
 181 > +;; General Public License for more details.\r
 182 > +;;\r
 183 > +;; You should have received a copy of the GNU General Public License\r
 184 > +;; along with Notmuch.  If not, see <http://www.gnu.org/licenses/>.\r
 185 > +;;\r
 186 > +;; Authors: Damien Cassou <damien.cassou@gmail.com>\r
 187 > +;;; Commentary:\r
 188 \r
 189 Since there is no commentary, you should leave out this header section.\r
 190 \r
 191 > +;;\r
 192 > +;;; Comments\r
 193 \r
 194 I'm not sure what this is, but it should probably also be omitted.\r
 195 \r
 196 > +;;\r
 197 > +;;; Code:\r
 198 > +;;\r
 199 > +\r
 200 > +(require 'cl)\r
 201 > +\r
 202 > +(defvar notmuch-tagger-formats\r
 203 \r
 204 notmuch-tag-formats?\r
 205 \r
 206 > +  `(("unread"\r
 207 > +     (:propertize "unread" face\r
 208 > +                  (:Foreground "red")))\r
 209 > +    ("flagged"\r
 210 > +     (:propertize "flagged" display\r
 211 > +                  (image :type svg :file\r
 212 > +                         ,(expand-file-name\r
 213 > +                           "resources/star.svg"\r
 214 > +                           (file-name-directory\r
 215 > +                            (or\r
 216 > +                             (locate-library "notmuch-tagger")\r
 217 > +                             (buffer-file-name))))\r
 218 \r
 219 Interesting.  Is this a standard way to locate resources?  (I've never\r
 220 had need to.)\r
 221 \r
 222 Since there are other icons here as well, perhaps the resources\r
 223 directory should be bound to a global variable so it's easy to construct\r
 224 other standard notmuch icon names?  You capture this in a function in\r
 225 patch 4, but that function doesn't obviously accomplish anything a\r
 226 simple variable wouldn't.\r
 227 \r
 228 Another possibility is that these icons could be included directly in\r
 229 the Elisp source, probably as simplified SVGs (your SVGs look like they\r
 230 could be cut down to 3 or 4 lines of XML easily) or as XPMs.  Besides\r
 231 skirting issues with resource location, this would make it trivial to\r
 232 alter their colors based on user preferences.\r
 233 \r
 234 > +                         :ascent center :mask heuristic))))\r
 235 > +  "Contains pairs of (KEY FORMAT) to format a tag matching KEY.\r
 236 > +\r
 237 > +KEY must be a string with a tag name. In the future, KEY could\r
 238 > +also be a regexp or list of keys to be matched against tags.\r
 239 \r
 240 This "in the future" comment doesn't really belong in a doc string,\r
 241 though it would make sense as a source comment in\r
 242 `notmuch-tagger-tag-format'.\r
 243 \r
 244 > +\r
 245 > +The default value set the unread tag to be red and the flagged\r
 246 > +tag to have a star picture attached. Those are just examples so\r
 247 > +you get an idea of what you can do.")\r
 248 > +\r
 249 > +(defun notmuch-tagger-tag-format (tag)\r
 250 > +  "Format TAG as a `mode-line-format' template.\r
 251 \r
 252 This makes it sounds like this function actually formats the template,\r
 253 which it doesn't.  Perhaps\r
 254 \r
 255   Return a `mode-line-format' template for tag TAG.\r
 256 \r
 257 and call it notmuch-tag-get-format?\r
 258 \r
 259 > +\r
 260 > +The format to apply to TAG is searched in\r
 261 > +`notmuch-tagger-formats'. If not found, the default\r
 262 > +`notmuch-tag-face' is used."\r
 263 > +  (let ((match (assoc tag notmuch-tagger-formats)))\r
 264 > +    (if match\r
 265 > +        (cadr match)\r
 266 > +      `(:propertize ,tag face notmuch-tag-face))))\r
 267 \r
 268 This changes how we use notmuch-tag-face.  For example, currently, if\r
 269 notmuch-tag-face has a background color, it will apply to the whole tag\r
 270 list, including the spaces between tags.  With this, it will only apply\r
 271 to the individual tags.  Also, if a tag does have a format, it will\r
 272 completely override notmuch-tag-face, rather than combining with it,\r
 273 which I think is undesirable.  Fixing this is a little tricky because\r
 274 Emacs makes it a pain to combine faces, but\r
 275 notmuch-combine-face-text-property already mostly implements this.  I'd\r
 276 suggest simply returning tag in the alternate case above (in which case\r
 277 you can simplify the above function down to just\r
 278  (assoc-default tag notmuch-tag-formats nil tag)\r
 279 ) and replacing the propertize calls that used to apply notmuch-tag-face\r
 280 with calls to this updated notmuch-combine-face-text-property:\r
 281 \r
 282 (defun notmuch-combine-face-text-property (start end face &optional below o=\r
 283 bject)\r
 284   "Combine FACE into the 'face text property between START and END.\r
 285 \r
 286 This function combines FACE with any existing faces between START\r
 287 and END in OBJECT (which defaults to the current buffer).\r
 288 Attributes specified by FACE take precedence over existing\r
 289 attributes unless BELOW is non-nil.  FACE must be a face name (a\r
 290 symbol or string), a property list of face attributes, or a list\r
 291 of these.  For convenience when applied to strings, this returns\r
 292 OBJECT."\r
 293 \r
 294   (let ((pos start))\r
 295     (while (< pos end)\r
 296       (let* ((cur (get-text-property pos 'face object))\r
 297              (next (next-single-property-change pos 'face object end))\r
 298              (new (if below\r
 299                       (append cur (list face))\r
 300                     (cons face cur))))\r
 301         (put-text-property pos next 'face new object)\r
 302         (setq pos next))))\r
 303   object)\r
 304 \r
 305 The new BELOW argument will let faces applied by tag formats override\r
 306 notmuch-tag-face even though notmuch-tag-face is applied afterward, and\r
 307 the new OBJECT argument will let this operate on strings.\r
 308 \r
 309 > +\r
 310 > +(defun notmuch-tagger-tags-format (tags)\r
 311 > +  "Format TAGS as a `mode-line-format' template."\r
 312 \r
 313 Same comment here.  Perhaps notmuch-tag-list-get-format and\r
 314 \r
 315   Return a `mode-line-format' template for tag list TAGS.\r
 316 \r
 317 > +  (notmuch-intersperse\r
 318 > +   (remove nil (mapcar #'notmuch-tagger-tag-format tags))\r
 319 \r
 320 The remove nil would be worth a comment.  It took me a long time to\r
 321 figure out that this was because tags can be hidden by formatting them\r
 322 as nil (which makes sense in retrospect).\r
 323 \r
 324 > +   " "))\r
 325 > +\r
 326 > +(defun notmuch-tagger-present-tags (tags)\r
 327 \r
 328 This doesn't actually present the tags.  If you like the renaming I\r
 329 suggested above, it would make sense to call this\r
 330 notmuch-tag-format-tag-list, since it actually does the formatting.\r
 331 \r
 332 > +  "Return a string that represent TAGS with their format."\r
 333 > +  (format-mode-line (notmuch-tagger-tags-format tags)))\r
 334 > +\r
 335 > +(provide 'notmuch-tagger)\r
 336 > +;;; notmuch-tagger.el ends here\r
 337 > diff --git a/emacs/notmuch.el b/emacs/notmuch.el\r
 338 > index c98a4fe..c607905 100644\r
 339 > --- a/emacs/notmuch.el\r
 340 > +++ b/emacs/notmuch.el\r
 341 > @@ -797,9 +797,8 @@ non-authors is found, assume that all of the authors =\r
 342 match."\r
 343 >      (notmuch-search-insert-authors format-string (plist-get result :auth=\r
 344 ors)))\r
 345 >=20=20\r
 346 >     ((string-equal field "tags")\r
 347 > -    (let ((tags-str (mapconcat 'identity (plist-get result :tags) " ")))\r
 348 > -      (insert (propertize (format format-string tags-str)\r
 349 > -                       'face 'notmuch-tag-face))))))\r
 350 > +    (let ((tags (plist-get result :tags)))\r
 351 > +      (insert (format format-string (notmuch-tagger-present-tags tags)))=\r
 352 ))))\r
 353 >=20=20\r
 354 >  (defun notmuch-search-show-result (result &optional pos)\r
 355 >    "Insert RESULT at POS or the end of the buffer if POS is null."\r
 356 > --=20\r
 357 > 1.7.10.4\r
 358 >\r
 359 > _______________________________________________\r
 360 > notmuch mailing list\r
 361 > notmuch@notmuchmail.org\r
 362 > http://notmuchmail.org/mailman/listinfo/notmuch\r