Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encode output inside mako cmd and write as binary #377

Closed
wants to merge 2 commits into from
Closed

Encode output inside mako cmd and write as binary #377

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba40dbe
Encode output inside mako cmd and write as binary
ni-balexand Feb 15, 2023
6cec49e
Merge pull request #1 from ni-balexand/ni-balexand-patch-1
ni-balexand Feb 15, 2023
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
6 changes: 5 additions & 1 deletion mako/cmd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@ def cmdline(argv=None):
_exit()
else:
if output_file:
open(output_file, "wt", encoding=output_encoding).write(rendered)
if output_encoding is None:
encoded_output = rendered.encode()
else:
encoded_output = rendered.encode(output_encoding)
open(output_file, "wb").write(encoded_output)
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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).

Copy link
Member

Choose a reason for hiding this comment

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

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

else:
sys.stdout.write(rendered)

Expand Down