Encode output inside mako cmd and write as binary

[ni-balexand]

This change is a proposed fix for #376

[zzzeek]

Id like to remove all binary mode from Mako as we are in python 3 now

[zzzeek]

please fix this on the read side instead; we should not be reading files in binary mode

[ni-balexand]

I was thinking about that but that type of change would be higher impact, i.e. it would affect users of mako.template

# Long-standing template.render behavior
template = mako.template.Template(filename="template_file")
rendered = template.render()
# now:
# rendered == 'Line 1\r\nLine 2\r\nLine 3\r\n'
# if we changed to read as text:
# rendered == 'Line 1\nLine 2\nLine 3\n'

I'd be more concerned about changing this behavior since it's established that mako does not do newline conversion.
I'm less concerned with the impact of changing things in mako.cmd, especially since the existing behavior is completely misaligned with any newline convention (i.e. the awkward \r\r\n sequences).

[zzzeek]

reading text mode on the front is as you mention not that feasible unless I want to no longer support "magic encoding comments", which I don't, but i dont have the appetite to rework that now.

the fix here is not the encoding, it's the newline conversion. we dont need it and it can be turned off directly without otherwise changing how text mode wants to write files:

diff --git a/mako/cmd.py b/mako/cmd.py
index 93a6733..deb8b95 100755
--- a/mako/cmd.py
+++ b/mako/cmd.py
@@ -86,12 +86,12 @@ def cmdline(argv=None):
 
     kw = dict(varsplit(var) for var in options.var)
     try:
-        rendered = template.render(**kw)
+        rendered = template.render_unicode(**kw)
     except:
         _exit()
     else:
         if output_file:
-            open(output_file, "wt", encoding=output_encoding).write(rendered)
+            open(output_file, "wt", newline="", encoding=output_encoding).write(rendered)
         else:
             sys.stdout.write(rendered)
 

can you do that here? thanks

[sqla-tester]

Mike Bayer has proposed a fix for this issue in the main branch:

switch to textmode in all cases https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/4450

[zzzeek]

reading text mode on the front is as you mention not that feasible unless I want to no longer support "magic encoding comments", which I don't, but i dont have the appetite to rework that now.

the fix here is not the encoding, it's the newline conversion. we dont need it and it can be turned off directly without otherwise changing how text mode wants to write files:

diff --git a/mako/cmd.py b/mako/cmd.py
index 93a6733..deb8b95 100755
--- a/mako/cmd.py
+++ b/mako/cmd.py
@@ -86,12 +86,12 @@ def cmdline(argv=None):
 
     kw = dict(varsplit(var) for var in options.var)
     try:
-        rendered = template.render(**kw)
+        rendered = template.render_unicode(**kw)
     except:
         _exit()
     else:
         if output_file:
-            open(output_file, "wt", encoding=output_encoding).write(rendered)
+            open(output_file, "wt", newline="", encoding=output_encoding).write(rendered)
         else:
             sys.stdout.write(rendered)
 

can you do that here? thanks

[zzzeek]

I dont understand this PR or my comments about "fix this on the read side". If this is for #376 why isn't it just the fix I proposed at #376 (comment) ? Changing Mako to not read from binary mode or whatever can be something else.

can we make a PR that fixes #376 in the most simple way possible, thanks.

[zzzeek]

looks like I requested that here at #377 (review) already, so this PR is stalled. We can reopen if someone has time to work on this or make a new PR against #376.