Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 20 additions & 14 deletions muttdown/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
import subprocess
import six

import markdown
import pynliner
# Use pandoc for markdown, and inline the CSS with battleworn tool:
import pypandoc
import premailer
import tempfile

from . import config
from . import __version__
Expand All @@ -27,24 +29,28 @@ def convert_one(part, config):
text = part.get_payload(decode=True)
if not isinstance(text, six.text_type):
# no, I don't know why decode=True sometimes fails to decode.
# it is because its decoding from different formats. first base64 then unicode ---guygma
text = text.decode('utf-8')
if not text.startswith('!m'):
return None
text = re.sub('\s*!m\s*', '', text, re.M)
f = tempfile.NamedTemporaryFile(suffix='_panmail.css')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

temporary files should always be used as context managers so that they get cleaned up on pypy

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bothered me as well -- to be sure. I did initially implement it that way. The reason I changed was to try and change less of the underlying code structure. I think handling the signature separately is where I ran into an awkward situation so found my way around it by just manually closing the file later (which also deletes it). Can do better on that though.

if config.css: f.write(config.css)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

never do one-line if statements in python

if '\n-- \n' in text:
pre_signature, signature = text.split('\n-- \n')
md = markdown.markdown(pre_signature, output_format="html5")
md += '\n<div class="signature" style="font-size: small"><p>-- <br />'
md += '<br />'.join(signature.split('\n'))
md += '</p></div>'
message = pypandoc.convert_text(pre_signature, 'html5', format='md', \
extra_args=["--css="+f.name, "--self-contained", "--metadata=pagetitle:'email'"])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

preferred style would be something like

message = pypandoc.convert_text(
    pre_signature, 'html5', format='md',
    extra_args=['--css=' + f.name, ...]
)

message += '\n<div class="signature" style="font-size: small"><p>-- \n<br />'
message += '<br />'.join(signature.split('\n'))
message += '</p></div>'
else:
md = markdown.markdown(text)
if config.css:
md = '<style>' + config.css + '</style>' + md
md = pynliner.fromString(md)
message = MIMEText(md, 'html', _charset="UTF-8")
message = pypandoc.convert_text(text, 'html5', format='md', \
extra_args=["--css="+f.name, "--self-contained", "--metadata=pagetitle:'email'"])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above comment regarding correct indentation for multiline function call arguments

message = premailer.transform(message) # In-line the CSS.
message = MIMEText(message, 'html', _charset="UTF-8")
f.close()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will no longer be necessary when you switch to a context manager

return message
except Exception:
except Exception:#silly. only need this to handle a certain exception differently from default.
raise
return None

Expand Down Expand Up @@ -160,10 +166,10 @@ def main():
if args.print_message:
print(rebuilt.as_string())
elif args.sendmail_passthru:
cmd = c.sendmail.split() + ['-f', args.envelope_from] + args.addresses
cmd = c.sendmail.split() + ['-G', '-i', '-f', args.envelope_from] + args.addresses
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-G would need to be controlled by a config flag since it would only be appropriate when using this as a postfix tool


proc = subprocess.Popen(cmd, stdin=subprocess.PIPE, shell=False)
proc.communicate(rebuilt.as_string())
proc.communicate(rebuilt.as_string().encode())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calling encode without an argument has pretty confusing behavior on a lot of systems since it tries to guess the default encoding

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguable, I think the default is actually the least confusing possible behavior. I would opt instead for just dropping support of legacy python, which is really the root cause of confusion around strings/bytes. Especially with the additional decode bound method that converts the html from base64 to usual text encoding.

return proc.returncode
else:
conn = smtp_connection(c)
Expand Down
4 changes: 2 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
long_description=long_description,
long_description_content_type="text/markdown",
install_requires=[
'Markdown>=2.0',
'PyYAML>=3.0',
'pynliner==0.8.0',
'pypandoc',
'premailer',
'six',
],
entry_points={
Expand Down