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.

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

Fix octal literal and long syntax for Python 3 #185

Closed
wants to merge 2 commits into from
Closed

Fix octal literal and long syntax for Python 3 #185

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 19, 2013

When I attempt to import jinja2 on Python 3.3, I get a SyntaxError:

Python 3.3.0 (default, Feb  3 2013, 16:59:54) 
[GCC 4.2.1 Compatible Apple LLVM 4.2 (clang-425.0.24)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import jinja2
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "./jinja2/__init__.py", line 33, in <module>
    from jinja2.environment import Environment, Template
  File "./jinja2/environment.py", line 580
    info.external_attr = 0755 << 16L
                            ^
SyntaxError: invalid token

This pull request corrects that syntax error by modifying the code in question to reflect changes to octal literal and long integer syntax made in Python 3.0.

The code appears to be compatible across Python versions, with all unit tests in versions 2.6, 2.7, and 3.3 passing. The line modified in this pull request causes a failure with Python 2.5.

Interestingly, while I was unable to import Jinja2 in Python 3.3 as a result of this bug, the syntax error was not caught when I ran unit tests with that version even though it was caught when I ran the unit tests with Python 2.5.

@masklinn
Copy link

For the octal, in order to keep 2.5 compat' wouldn't it be better to parse it as an octal string if you want to keep octal? int('755', 8) should work in all Python versions

@ghost
Copy link
Author

ghost commented Apr 16, 2013

@masklinn You're right - I've updated my branch to reflect that. Thanks!

@mitsuhiko
Copy link
Contributor

Changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants