The fourth commit tries to address the issue raised in id:87k0sxa6c3.fsf@fifthhorseman.net. The output of "notmuch show --format=sexp --format-version=4" may contain `:content-type' entries with `nil' as the value, when it fails to detect the correct value. Account for that in a few places where we would otherwise risk a type error. The other three commits cleanup the code I had to look at to do the above, because I just can't help myself. This does not apply without first applying id:20201214162401.19569-1-jonas@bernoul.li. Jonas Jonas Bernoulli (4): emacs: notmuch-mua-add-more-hidden-headers: use local binding emacs: notmuch-show--register-cids: fix names of bindings emacs: notmuch-show--get-cid-content: cosmetics emacs: avoid type errors due to nil as content-type emacs/notmuch-lib.el | 23 ++++++++++++---------- emacs/notmuch-mua.el | 7 +++---- emacs/notmuch-show.el | 46 +++++++++++++++++++++---------------------- 3 files changed, 39 insertions(+), 37 deletions(-) -- 2.30.0
--- emacs/notmuch-mua.el | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el index 08c73c16..bbf059a2 100644 --- a/emacs/notmuch-mua.el +++ b/emacs/notmuch-mua.el @@ -217,11 +217,10 @@ (defun notmuch-mua-add-more-hidden-headers () (defun notmuch-mua-reply-crypto (parts) "Add mml sign-encrypt flag if any part of original message is encrypted." (cl-loop for part in parts - if (notmuch-match-content-type (plist-get part :content-type) - "multipart/encrypted") + for type = (plist-get part :content-type) + if (notmuch-match-content-type type "multipart/encrypted") do (mml-secure-message-sign-encrypt) - else if (notmuch-match-content-type (plist-get part :content-type) - "multipart/*") + else if (notmuch-match-content-type type "multipart/*") do (notmuch-mua-reply-crypto (plist-get part :content)))) ;; There is a bug in Emacs' message.el that results in a newline -- 2.30.0
--- emacs/notmuch-show.el | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index fdf4ab3c..21133611 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -581,13 +581,13 @@ (defun notmuch-show--register-cids (msg part) ;; alternative (even if we can't render it). (push (list content-id msg part) notmuch-show--cids))) ;; Recurse on sub-parts - (pcase-let ((`(,content ,type) + (pcase-let ((`(,type ,subtype) (split-string (downcase (plist-get part :content-type)) "/"))) - (cond ((equal content "multipart") + (cond ((equal type "multipart") (mapc (apply-partially #'notmuch-show--register-cids msg) (plist-get part :content))) - ((and (equal content "message") - (equal type "rfc822")) + ((and (equal type "message") + (equal subtype "rfc822")) (notmuch-show--register-cids msg (car (plist-get (car (plist-get part :content)) :body))))))) -- 2.30.0
--- emacs/notmuch-show.el | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 21133611..8c846fb2 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -599,16 +599,13 @@ (defun notmuch-show--get-cid-content (cid) into the current buffer. CID must be a raw content ID, without enclosing angle brackets, a cid: prefix, or URL encoding. This will return nil if the CID is unknown or cannot be retrieved." - (let ((descriptor (cdr (assoc cid notmuch-show--cids)))) - (when descriptor - (let* ((msg (car descriptor)) - (part (cadr descriptor)) - ;; Request caching for this content, as some messages - ;; reference the same cid: part many times (hundreds!). - (content (notmuch-get-bodypart-binary - msg part notmuch-show-process-crypto 'cache)) - (content-type (plist-get part :content-type))) - (list content content-type))))) + (when-let ((descriptor (cdr (assoc cid notmuch-show--cids)))) + (pcase-let ((`(,msg ,part) descriptor)) + ;; Request caching for this content, as some messages + ;; reference the same cid: part many times (hundreds!). + (list (notmuch-get-bodypart-binary + msg part notmuch-show-process-crypto 'cache) + (plist-get part :content-type))))) (defun notmuch-show-setup-w3m () "Instruct w3m how to retrieve content from a \"related\" part of a message." -- 2.30.0
The output of "notmuch show --format=sexp --format-version=4" may contain `:content-type' entries with `nil' as the value, when it fails to detect the correct value. Account for that in a few places where we would otherwise risk a type error. Note that `string=' does not choke on `nil' because it uses the `symbol-name' when encountering a symbol. --- emacs/notmuch-lib.el | 23 +++++++++++++---------- emacs/notmuch-show.el | 29 ++++++++++++++++------------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index bc550dc2..c7bb2091 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -552,16 +552,19 @@ (defun notmuch-plist-delete (plist property) ;;; MML Utilities (defun notmuch-match-content-type (t1 t2) - "Return t if t1 and t2 are matching content types, taking wildcards into account." - (let ((st1 (split-string t1 "/")) - (st2 (split-string t2 "/"))) - (if (or (string= (cadr st1) "*") - (string= (cadr st2) "*")) - ;; Comparison of content types should be case insensitive. - (string= (downcase (car st1)) - (downcase (car st2))) - (string= (downcase t1) - (downcase t2))))) + "Return t if t1 and t2 are matching content types. +Take wildcards into account." + (and (stringp t1) + (stringp t2) + (let ((st1 (split-string t1 "/")) + (st2 (split-string t2 "/"))) + (if (or (string= (cadr st1) "*") + (string= (cadr st2) "*")) + ;; Comparison of content types should be case insensitive. + (string= (downcase (car st1)) + (downcase (car st2))) + (string= (downcase t1) + (downcase t2)))))) (defvar notmuch-multipart/alternative-discouraged '(;; Avoid HTML parts. diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 8c846fb2..ba93febb 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -581,16 +581,17 @@ (defun notmuch-show--register-cids (msg part) ;; alternative (even if we can't render it). (push (list content-id msg part) notmuch-show--cids))) ;; Recurse on sub-parts - (pcase-let ((`(,type ,subtype) - (split-string (downcase (plist-get part :content-type)) "/"))) - (cond ((equal type "multipart") - (mapc (apply-partially #'notmuch-show--register-cids msg) - (plist-get part :content))) - ((and (equal type "message") - (equal subtype "rfc822")) - (notmuch-show--register-cids - msg - (car (plist-get (car (plist-get part :content)) :body))))))) + (when-let ((type (plist-get part :content-type))) + (pcase-let ((`(,type ,subtype) + (split-string (downcase type) "/"))) + (cond ((equal type "multipart") + (mapc (apply-partially #'notmuch-show--register-cids msg) + (plist-get part :content))) + ((and (equal type "message") + (equal subtype "rfc822")) + (notmuch-show--register-cids + msg + (car (plist-get (car (plist-get part :content)) :body)))))))) (defun notmuch-show--get-cid-content (cid) "Return a list (CID-content content-type) or nil. @@ -948,7 +949,8 @@ (defun notmuch-show-lazy-part (part-args button) (defun notmuch-show-mime-type (part) "Return the correct mime-type to use for PART." - (let ((content-type (downcase (plist-get part :content-type)))) + (when-let ((content-type (plist-get part :content-type))) + (setq content-type (downcase content-type)) (or (and (string= content-type "application/octet-stream") (notmuch-show-get-mime-type-of-application/octet-stream part)) (and (string= content-type "inline patch") @@ -988,7 +990,7 @@ (defun notmuch-show-insert-bodypart (msg part depth &optional hide) HIDE determines whether to show or hide the part and the button as follows: If HIDE is nil, show the part and the button. If HIDE is t, hide the part initially and show the button." - (let* ((content-type (downcase (plist-get part :content-type))) + (let* ((content-type (plist-get part :content-type)) (mime-type (notmuch-show-mime-type part)) (nth (plist-get part :id)) (long (and (notmuch-match-content-type mime-type "text/*") @@ -1000,7 +1002,8 @@ (defun notmuch-show-insert-bodypart (msg part depth &optional hide) ;; the first (or only) part if this is text/plain. (button (and (funcall notmuch-show-insert-header-p-function part hide) (notmuch-show-insert-part-header - nth mime-type content-type + nth mime-type + (and content-type (downcase content-type)) (plist-get part :filename)))) ;; Hide the part initially if HIDE is t, or if it is too long ;; and we have a button to allow toggling. -- 2.30.0
On Sunday, 2021-01-10 at 19:47:18 +01, Jonas Bernoulli wrote: > The fourth commit tries to address the issue raised in > id:87k0sxa6c3.fsf@fifthhorseman.net. > > The output of "notmuch show --format=sexp --format-version=4" > may contain `:content-type' entries with `nil' as the value, > when it fails to detect the correct value. Account for that > in a few places where we would otherwise risk a type error. > > The other three commits cleanup the code I had to look at to do > the above, because I just can't help myself. > > This does not apply without first applying > id:20201214162401.19569-1-jonas@bernoul.li. I don't seem to have this one, but that could be a problem at my end. Anyway, for the series... Reviewed-by: David Edmondson <dme@dme.org> > > Jonas > > Jonas Bernoulli (4): > emacs: notmuch-mua-add-more-hidden-headers: use local binding > emacs: notmuch-show--register-cids: fix names of bindings > emacs: notmuch-show--get-cid-content: cosmetics > emacs: avoid type errors due to nil as content-type > > emacs/notmuch-lib.el | 23 ++++++++++++---------- > emacs/notmuch-mua.el | 7 +++---- > emacs/notmuch-show.el | 46 +++++++++++++++++++++---------------------- > 3 files changed, 39 insertions(+), 37 deletions(-) > > -- > 2.30.0 > _______________________________________________ > notmuch mailing list -- notmuch@notmuchmail.org > To unsubscribe send an email to notmuch-leave@notmuchmail.org dme. -- It was a perfectly good grand piano.
David Edmondson <dme@dme.org> writes: > On Sunday, 2021-01-10 at 19:47:18 +01, Jonas Bernoulli wrote: > >> The fourth commit tries to address the issue raised in >> id:87k0sxa6c3.fsf@fifthhorseman.net. >> >> The output of "notmuch show --format=sexp --format-version=4" >> may contain `:content-type' entries with `nil' as the value, >> when it fails to detect the correct value. Account for that >> in a few places where we would otherwise risk a type error. >> >> The other three commits cleanup the code I had to look at to do >> the above, because I just can't help myself. >> >> This does not apply without first applying >> id:20201214162401.19569-1-jonas@bernoul.li. > > I don't seem to have this one, but that could be a problem at my end. I am sending you a copy of the original cover letter and I have send v2 of the series earlier today, starting at id:20210110140112.25930-1-jonas@bernoul.li. > Anyway, for the series... > > Reviewed-by: David Edmondson <dme@dme.org> > >> >> Jonas >> >> Jonas Bernoulli (4): >> emacs: notmuch-mua-add-more-hidden-headers: use local binding >> emacs: notmuch-show--register-cids: fix names of bindings >> emacs: notmuch-show--get-cid-content: cosmetics >> emacs: avoid type errors due to nil as content-type >> >> emacs/notmuch-lib.el | 23 ++++++++++++---------- >> emacs/notmuch-mua.el | 7 +++---- >> emacs/notmuch-show.el | 46 +++++++++++++++++++++---------------------- >> 3 files changed, 39 insertions(+), 37 deletions(-) >> >> -- >> 2.30.0 >> _______________________________________________ >> notmuch mailing list -- notmuch@notmuchmail.org >> To unsubscribe send an email to notmuch-leave@notmuchmail.org > > dme. > -- > It was a perfectly good grand piano.
On Sun, Jan 10 2021, Jonas Bernoulli wrote: > The fourth commit tries to address the issue raised in > id:87k0sxa6c3.fsf@fifthhorseman.net. > > The output of "notmuch show --format=sexp --format-version=4" > may contain `:content-type' entries with `nil' as the value, > when it fails to detect the correct value. Account for that > in a few places where we would otherwise risk a type error. > > The other three commits cleanup the code I had to look at to do > the above, because I just can't help myself. > > This does not apply without first applying > id:20201214162401.19569-1-jonas@bernoul.li. Perhaps this series still applies on top of that series from 2020-12-14, but perhaps one should apply it on top of series (from 2021-01-10): id:20210110140112.25930-2-jonas@bernoul.li Meaning emails in range id:20210110140112.25930-2-jonas@bernoul.li ... id:20210110140112.25930-37-jonas@bernoul.li (also visible in https://nmbug.notmuchmail.org/status/) Note that messages 20210110140112.25930-11-jonas@bernoul.li and 20210110140112.25930-12-jonas@bernoul.li have base64 -encoded content, with CRLF line endings so those don't apply as is, but CR's from the encoded content must be deleted. I used a hack to do so, appended to the end of this email; David has done something that is more robust, and perhaps from that we could get a tool to be added in devel/ which could do this in the future. Both of these series applied cleanly, and I will start using those in my emacs mua. I like these changes and hope these can be applied soon. > > Jonas Tomi Usage: ./rmcr-base64.pl < input.mbox > output.mbox --8<----8<----8<----8<----8<----8<----8<----8<-- #!/usr/bin/perl # -*- mode: cperl; cperl-indent-level: 4 -*- use 5.8.1; use strict; use warnings; use MIME::Base64; while (<STDIN>) { print $_; if (/^Content-Transfer-Encoding: base64/) { while (<STDIN>) { print $_; last if /^\s*$/ } my @lines; while (<STDIN>) { last if /^\s*$/; push @lines, $_ } my $decoded = decode_base64 join('', @lines); $decoded =~ tr/\r//d; print(encode_base64 $decoded); print "\n" } }
Tomi Ollila <tomi.ollila@iki.fi> writes:
> Note that messages
>
> 20210110140112.25930-11-jonas@bernoul.li and
> 20210110140112.25930-12-jonas@bernoul.li
>
> have base64 -encoded content, with CRLF line endings
> so those don't apply as is, but CR's from the encoded
> content must be deleted.
Do you know why git would decide to send these commits that way?
Maybe we should try to keep it from doing that? I didn't write
down the prompt but git actually asked something along the line
of "for these two messages you have to select the base64 encoding,
does utf-8 work for you?".
Jonas
On Tue, Jan 12 2021, Jonas Bernoulli wrote: > Tomi Ollila <tomi.ollila@iki.fi> writes: >> Note that messages >> >> 20210110140112.25930-11-jonas@bernoul.li and >> 20210110140112.25930-12-jonas@bernoul.li >> >> have base64 -encoded content, with CRLF line endings >> so those don't apply as is, but CR's from the encoded >> content must be deleted. > > Do you know why git would decide to send these commits that way? I do know: In my case: - git format-patch ... - in one of the patch files there are utf-8 characters - git send-email (to mailing list and to tomi.ollila@iki.fi) - first stop in email is in smtp.iki.fi (which I use to send email) - the email that returns directly from iki.fi does not have base64 encoded content *nor* CR's in email body - the mail that is sent to notmuchmail.org, will contain CRLF line endings; either smtp.iki.fi adds those or notmuchmail.org smtpd added those - the mailman in notmuchmail.org notices that there is 8bit content in email body, and decides to base64-encode the whole content with CRLF line endings. - git am, the --no-keep-cr option (default) if not effective in "embedded" base64-encoded content. That particular feature is just for that purpose that smtp content should have CRLF line endings and the files those patches refer to often don't have (--keep-cr can be used if CRLF file endings are expected). It is a "feature" that in base64-encoded content is handled as verbatim by git-am, git-apply and so on. For reference: the email https://www.mail-archive.com/notmuch@notmuchmail.org/msg50113.html got directly (via-iki-smtpd) and via mailing list (via-notmuch-mailman) has the following headers (among other headers) in my mail storage (and as said above, the one got directly did not have CRLF line endings) $ grep Content via-iki-smtpd via-notmuch-mailman via-iki-smtpd:Content-Type: text/plain; charset=UTF-8 via-iki-smtpd:Content-Transfer-Encoding: 8bit via-notmuch-mailman:Content-Type: text/plain; charset="utf-8" via-notmuch-mailman:Content-Transfer-Encoding: base64 > Maybe we should try to keep it from doing that? I didn't write > down the prompt but git actually asked something along the line > of "for these two messages you have to select the base64 encoding, > does utf-8 work for you?". Right, the base64 encoding can be used when using git send-email, but how many knows to do that :) (Tried to look what option is it, but man git-send-email gave me manpage of version 1.7.1 >;/). Other options: - modify mailman (David Bremner contacted mailman mailing list, they don't think that is a bug (well IMO is it not a bug, but...) anyway one could hack locally ;/ (or the smtpd frontend if there is any) - modify git-am (based on Julio Hamano's comment on some mailing list something like if there is clear and sound implementation -- was it some filter option then something could get in... (or something)) - based on irclogs (2020-08) David has tool to be offered to (debian)-mailscripts to filter CR's out from base64-encoded content. -- which I think is better place as this problem isn't notmuch specific (although having such a tool in devel/ would be more convenient for me) > > Jonas Tomi
Jonas Bernoulli <jonas@bernoul.li> writes:
> The fourth commit tries to address the issue raised in
> id:87k0sxa6c3.fsf@fifthhorseman.net.
Series applied to master