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

Feature/long integer with underscores #923

Merged
merged 2 commits into from Jul 23, 2019

Conversation

@CleoQc
Copy link
Contributor

CleoQc commented Nov 13, 2018

It is now possible to use the '_' character to make long integers and floats more easily understandable by a user.
100_000_000_000 for example, or 100_000_000_000.000_000_001

I don't think this closes any issue but since I had my hands in regex, it was a good timing,

@CleoQc

This comment has been minimized.

Copy link
Contributor Author

CleoQc commented Nov 13, 2018

@davidism : how would you prefer to handle this? This feature is limited to Python 3..6 and up. Currently it doesn't degrade nicely but I could fix that. Or ignore it.
Do you still want to support versions older than 3.6? I'm guessing yes.

I could make the regex dependent on the python version. I'm open to suggestions but I won't put any more work into this one until I get your feedback.

@davidism

This comment has been minimized.

Copy link
Member

davidism commented Nov 13, 2018

We could support the underscore syntax for any version by removing them before calling literal_eval. Seems reasonable enough to do that.

@CleoQc

This comment has been minimized.

Copy link
Contributor Author

CleoQc commented Nov 13, 2018

Good point and should be simple enough to do.

@CleoQc

This comment has been minimized.

Copy link
Contributor Author

CleoQc commented Nov 13, 2018

Also of note, this PR contains the changes to #922 as they are literally on the same lines.

@CleoQc

This comment has been minimized.

Copy link
Contributor Author

CleoQc commented Nov 14, 2018

Finally passes all tests. Let me know if it's acceptable, please.

@CleoQc

This comment has been minimized.

Copy link
Contributor Author

CleoQc commented Nov 14, 2018

I've changed test_float and test_int but I'm thinking we should probably have a test_template that actually tests the regex.

jinja2/lexer.py Outdated Show resolved Hide resolved
tests/test_filters.py Outdated Show resolved Hide resolved
docs/templates.rst Outdated Show resolved Hide resolved
docs/templates.rst Outdated Show resolved Hide resolved
@davidism

This comment has been minimized.

Copy link
Member

davidism commented Nov 24, 2018

I'll get to this eventually! I'm just focused on Werkzeug right now. :-)

CleoQc and others added 2 commits Nov 13, 2018
add changelog
clean up docs
parametrize tests
explain float regex
@davidism davidism force-pushed the CleoQc:feature/long_integer_with_underscores branch from 0ed6689 to 733851e Jul 23, 2019
@davidism

This comment has been minimized.

Copy link
Member

davidism commented Jul 23, 2019

Rebased onto current master + #922 changes. Explained the regex with verbose mode. Allow '_' in any part, which matches Python's grammar. Moved the tests out of filters and into lexer, since that's what this was about, but cleaned up both sets of tests with pytest.mark.parametrize.

@davidism davidism added this to the 2.11.0 milestone Jul 23, 2019
@davidism davidism merged commit 8e3c0e7 into pallets:master Jul 23, 2019
10 checks passed
10 checks passed
Tests Build #20190723.3 succeeded
Details
Tests (Job Docs) Job Docs succeeded
Details
Tests (Job PyPy 3 Linux) Job PyPy 3 Linux succeeded
Details
Tests (Job Python 2.7 Linux) Job Python 2.7 Linux succeeded
Details
Tests (Job Python 2.7 Windows) Job Python 2.7 Windows succeeded
Details
Tests (Job Python 3.5 Linux) Job Python 3.5 Linux succeeded
Details
Tests (Job Python 3.6 Linux) Job Python 3.6 Linux succeeded
Details
Tests (Job Python 3.7 Linux) Job Python 3.7 Linux succeeded
Details
Tests (Job Python 3.7 Mac) Job Python 3.7 Mac succeeded
Details
Tests (Job Python 3.7 Windows) Job Python 3.7 Windows succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.