Much nicer now that it uses the mm stuff. On Mon, 16 Jan 2012 11:13:23 -0700, Adam Wolfe Gordon <awg+notmuch at xvx.ca> wrote: > +(defun find-parts (parts type)
Sorry for being a nuisance - this needs a name that indicates that it relates to notmuch. How about `notmuch-parts-filter-by-type'? > + "Return a list of message parts with the given type" > + (delq nil (mapcar (lambda (part) > + (if (string= (cdr (assq 'content-type part)) type) > + (cdr (assq 'content part)))) > + parts))) '(delq nil ...)' can more readably be implemented using something like: (loop for part in parts if (string= (cdr (assq 'content-type part)) type) collect (cdr (assq 'content part))) (untested). > +(defun notmuch-mua-insert-part-quoted (part) > + (save-restriction > + (narrow-to-region (point) (point)) > + (insert part) > + (goto-char (point-min)) > + (perform-replace "^" "> " nil t nil) Narrowing to '(point) (point)' seems a bit weird and using `perform-replace' is discouraged in programs. It would be more normal to use a loop of `re-search-forward' and `replace-match' with a limit after the insertion. Something like: (let ((start (point)) limit) (insert part) (setq limit (point)) (goto-char start) (while (re-search-forward "^" limit t) (replace-match "> ")) ... (untested). > + (insert "\n") > + (set-buffer-modified-p nil))) Is this newline always required? Is it the cause of the spurious blank line down below? > +(defun notmuch-mua-parse-html-part (part) > + (with-temp-buffer > + (insert part) > + (let ((handle (mm-make-handle (current-buffer) (list "text/html"))) > + (end-of-orig (point-max))) > + (mm-display-part handle) > + (kill-region (point-min) end-of-orig) > + (fill-region (point-min) (point-max)) > + (buffer-substring (point-min) (point-max))))) `kill-region' will save content in the kill ring. Was that intended? (Maybe `delete-region' instead?) > (defun notmuch-mua-reply (query-string &optional sender reply-all) > ... > + (insert (format "On %s, %s wrote:\n" > + (cdr (assq 'date original-headers)) > + (cdr (assq 'from original-headers)))) I wonder whether emacs should be regenerating this or not. I'm okay with it, but previous discussion was that it should remain the responsibility of the CLI. > + (if (null plain-parts) > + (mapc (lambda (part) (notmuch-mua-insert-part-quoted > (notmuch-mua-parse-html-part part))) html-parts) > + (mapc (lambda (part) (notmuch-mua-insert-part-quoted part)) > plain-parts)) Flip the 'then' and 'else' clauses to get rid of the `null'? > --- a/test/emacs > +++ b/test/emacs > @@ -270,6 +270,7 @@ Fcc: $(pwd)/mail/sent > --text follows this line-- > On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite at > notmuchmail.org> wrote: > > This is a test that messages are sent via SMTP > +> It would be good if you could get rid of this trailing blank line. -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120117/40c6bf84/attachment.pgp>