[PATCH v2 4/4] emacs: Use the new JSON reply format.
David Edmondson
dme at dme.org
Tue Jan 17 01:04:39 PST 2012
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>
More information about the notmuch
mailing list