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

update ** and // to support int/float literals #234

Conversation

bkrainer
Copy link
Contributor

@bkrainer bkrainer commented Sep 6, 2018

These changes address issue #169.

The current logic for // and ** specifies that the operands need to be IDENTIFIER symbols. This works when jinjava variables are used, but fails for actual numerical values (as these are INTEGER symbols, not IDENTIFIER).

The fix for this was to remove the special // and ** logic in ExtendedParser.value and instead rely on the existing extension framework via the pre-existing calls to Parser.getExtensionHandler

@codecov-io
Copy link

codecov-io commented Sep 6, 2018

Codecov Report

Merging #234 into master will increase coverage by 0.14%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master    #234      +/-   ##
===========================================
+ Coverage     71.46%   71.6%   +0.14%     
- Complexity     1376    1378       +2     
===========================================
  Files           218     218              
  Lines          4356    4350       -6     
  Branches        691     689       -2     
===========================================
+ Hits           3113    3115       +2     
+ Misses         1001     999       -2     
+ Partials        242     236       -6
Impacted Files Coverage Δ Complexity Δ
...ava/com/hubspot/jinjava/el/ext/ExtendedParser.java 84.8% <100%> (+0.51%) 58 <0> (-2) ⬇️
...a/com/hubspot/jinjava/el/ext/TruncDivOperator.java 70% <0%> (+15%) 12% <0%> (+2%) ⬆️
...va/com/hubspot/jinjava/el/ext/PowerOfOperator.java 75% <0%> (+15%) 13% <0%> (+2%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update def74f9...5860b7d. Read the comment docs.

Copy link
Contributor

@boulter boulter left a comment

Choose a reason for hiding this comment

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

nice job!

@bkrainer
Copy link
Contributor Author

bkrainer commented Sep 6, 2018

@boulter could you add me as a contributor to this repo? I don't have permissions to merge the PR

@mattcoley
Copy link
Collaborator

Done

@bkrainer bkrainer merged commit e62e2d3 into HubSpot:master Sep 7, 2018
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.

4 participants