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

Compile noninteger TR35 operands to zeroes when emitting Gettext #295

Merged
merged 4 commits into from Feb 8, 2016

Conversation

@codecov-io
Copy link

Current coverage is 88.47%

Merging #295 into master will increase coverage by +0.09% as of 8a1e889

@@            master    #295   diff @@
======================================
  Files           23      23       
  Stmts         3626    3635     +9
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit           3205    3216    +11
  Partial          0       0       
+ Missed         421     419     -2

Review entire Coverage Diff as of 8a1e889

Powered by Codecov. Updated on successful CI builds.

@akx
Copy link
Member Author

akx commented Feb 3, 2016

@imankulov: If you're around, could you give this a review?

@imankulov
Copy link
Contributor

@akx I'm sorry for delay.

I finally managed to test it, and it works as expected! Rules for Russian and other languages involving variables other than n, can be simplified, of course, but I guess this is not the goal of this PR, and they are technically correct.

That's how Russian rule looks like, for example:

nplurals=4; plural=(((((0 == 0)) && (((n % 10) >= 2 && (n % 10) <= 4))) && (!(((n % 100) >= 12 && (n % 100) <= 14)))) ? 1 : (((((0 == 0)) && (((n % 10) == 0))) || (((0 == 0)) && (((n % 10) >= 5 && (n % 10) <= 9)))) || (((0 == 0)) && (((n % 100) >= 11 && (n % 100) <= 14)))) ? 2 : ((((0 == 0)) && (((n % 10) == 1))) && (!(((n % 100) == 11)))) ? 0 : 3)

Have nothing against merging it. Thank you for the fix!

@akx
Copy link
Member Author

akx commented Feb 8, 2016

Great! Yeah, expression simplification is probably a little out of scope (though I have to admit it would be interesting...)

Merging this, thank you!

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

Successfully merging this pull request may close these issues.

None yet

5 participants