akissinger
Use notmuch to get attachment contents
Simplifies the code and works with PGP encrypted messages (where the previous logic did not find any attachments).
Due to letting notmuch handle the attachment extraction, this should hopefully also supersede #53 without regressing. Unfortunately I was unable to replicate the issue described there, even with a long attachment filename and that commit reverted.
@hbog do you perhaps still have the mail that prompted that bugfix, so you could double-check that it works fine after my change still? (we should probably have an automated testsuite for Dodo at some point, but that's a whole chapter on its own...)
This conflicts due to:
- #60
and it's not quite clear to me either how notmuch handles that internally. @laarmen do you have an example message you could use to test this with and see if just using notmuch's stdout 1:1 works fine encoding-wise?
@hbog @laarmen gentle ping - would be nice to make sure your bugfixes don't regress with this.
I don't remember which email was the culprit for the issue, but I'm fine merging this. Worst case scenario I'll patch notmuch directly 😁
On Mon, 7 Oct 2024, 13:47 Florian Bruhin, @.***> wrote:
@hbog https://github.com/hbog @laarmen https://github.com/laarmen gentle ping - would be nice to make sure your bugfixes don't regress with this.
— Reply to this email directly, view it on GitHub https://github.com/akissinger/dodo/pull/79#issuecomment-2396703234, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEQ4NMD4HQLCYKFNDLWKSDZ2JYEFAVCNFSM6AAAAABOLMUAE2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJWG4YDGMRTGQ . You are receiving this because you were mentioned.Message ID: @.***>
I agree that it will supersede #53 without regressing because this PR no longer uses the python email module for processing the attachment contents. #53 was due to a bug in in the (deprecated) python email.message class, which was replaced in #53 by email.EmailMessage during attachment handling.
is this PR still alive? If yes, I'll merge it once you fix the conflict.
Thanks for the nudge, updated now! I still use this in my branch, and it's been working fine for me so far FWIW.