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

Implement gettext expression evaluator #11993

Closed
nijel opened this issue Feb 19, 2016 · 25 comments
Closed

Implement gettext expression evaluator #11993

nijel opened this issue Feb 19, 2016 · 25 comments
Assignees
Labels
enhancement A feature request for improving phpMyAdmin newbie

Comments

@nijel
Copy link
Contributor

nijel commented Feb 19, 2016

For gettext we need something what is able to evaluate it's plural expressions and not use eval() (see #6363), unfortunately I was not able to find single arithmetic expression evaluation library for PHP which supports ternary operator, but they tend to support dozen of other stuff we don't need like functions. Therefore I think it will be better to implement own code for this particular use case. The final goal is to remove eval usage in gettext library.

Example expression we need to evaluate for any arbitrary value of n: n==0 ? 0 : n==1 ? 1 : n==2 ? 2 : n%100>=3 && n%100<=10 ? 3 : n%100>=11 ? 4 : 5

The syntax is quite limited:

  • n variable
  • ternary operator ? :
  • comparisons == < > <= => !=
  • division and modulo / %
  • numbers

PS: More plural examples can be found in the Localization Guide.

@nijel nijel added enhancement A feature request for improving phpMyAdmin newbie labels Feb 19, 2016
@nijel
Copy link
Contributor Author

nijel commented Feb 19, 2016

Of course viable alternative is finding composer package which can support ternary operator (or where you implement it).

@zaverichintan
Copy link
Contributor

@nijel What about this?
as per your comment-
As for Gettext at least one problem is still there - once the .mo file is loaded, PHP caches it and it won't see any updates until restart (or cache expiry, what does not seem to happen much often, probably depends on number of open .mo files).

@nijel
Copy link
Contributor Author

nijel commented Feb 19, 2016

That's problem for using PHP's builtin gettext, that's why we need the one we have.

@zaverichintan
Copy link
Contributor

So we can find/make eval() function which will be replace the eval used inside the PHP's builtin gettext ?

@nijel
Copy link
Contributor Author

nijel commented Feb 19, 2016

No, in our library.

@zaverichintan
Copy link
Contributor

Thanx, got it. I will try if i can get some available function or implement our own.

@wadhwasahil
Copy link

where in the code is eval() called?
I can make the function for evaluating the ? operator.
Thanks.

@udan11
Copy link
Contributor

udan11 commented Feb 20, 2016

Tip: This can be implemented quite easily using the shunting-yard algorithm, but for the ternary operator you will have to hack it a little bit.

Edit: The shunting-yard algorithm will only produce an abstract syntax tree or reverse Polish notation which can be easily evaluated.

@naveenshukla
Copy link

I haven't worked on any issue or enhancement yet. I would like to work on this enhancement :-)

@zaverichintan
Copy link
Contributor

@udan11 can you please give a example of the implementation

@nijel
Copy link
Contributor Author

nijel commented Feb 22, 2016

I've started to separate the gettext code to separate module, so implementing this might rather go there: https://github.com/phpmyadmin/motranslator

@udan11
Copy link
Contributor

udan11 commented Feb 23, 2016

@zaverichintan There are plenty of implementation on StackOverflow. You might want to start there.

@naveenshukla
Copy link

couldn't we have semicolon or assignment operator in the statement we have to evaluate? What I found on stackoverflow is this [http://stackoverflow.com/questions/13681293/how-can-i-incorporate-ternary-operators-into-a-precedence-climbing-algorithm/18910078#18910078]

@nijel
Copy link
Contributor Author

nijel commented Feb 24, 2016

I think it's enough to evaluate the expression part, no need to understand semicolon and assignment, these are easy to parse separately (and existing code in motranslator already does this).

Let's take Arabic as an example - the expression which we need to evaluate is (n==0 ? 0 : n==1 ? 1 : n==2 ? 2 : n%100>=3 && n%100<=10 ? 3 : n%100>=11 ? 4 : 5) for any given value of n.

@naveenshukla
Copy link

@nijel Why php-gettext library is removed? I was implementing code for expression evaluator.

@nijel
Copy link
Contributor Author

nijel commented Feb 25, 2016

See above comments, it was moved to separate repository - https://github.com/phpmyadmin/motranslator , related code is still same: https://github.com/phpmyadmin/motranslator/blob/master/src/Translator.php

@naveenshukla
Copy link

@nijel I wrote a code which takes expression like one given in example as string and prints the results. You can find code here : https://github.com/shuklank/testprojects/blob/master/php/gettext_eval.php . I want to know how to integrate this code into main library. I have only considered plural forms given in the link.

@nijel
Copy link
Contributor Author

nijel commented Feb 29, 2016

Few thoughts before this can be integrated:

  • the code is horribly ineffective, calling substr($str,$i,1) for every comparison really hurts
  • you can't rely on ctype extension being available
  • it lacks documentation
  • the code should be wrapped in a functions or class and do not use global variables

@naveenshukla
Copy link

@nijel Thanks for suggestion. Can I improve this code or we have to write fresh code again? I want to know if use of array instead of substr will be effective.

@nijel
Copy link
Contributor Author

nijel commented Feb 29, 2016

Of course you can improve it :-). It doesn't matter if array access of substr call, but once you have dozen of these, it's better to store it as variable $current = substr($str,$i,1)...

@naveenshukla
Copy link

thanks for motivation :-)
Here are the changes I'm thinking of

  1. the one you said $current = substr($str,$i,1)
  2. use of variable in the string (n). currently I was passing only numbers
  3. making code object oriented
  4. simple documentation
  5. instead of ctype , using comparison for all digits

@nijel nijel self-assigned this Jul 23, 2016
@nijel
Copy link
Contributor Author

nijel commented Jul 23, 2016

Closing in favor of phpmyadmin/motranslator#5 (as this is where this code currently lives).

@nijel nijel closed this as completed Jul 29, 2016
@nijel
Copy link
Contributor Author

nijel commented Oct 12, 2016

JFYI: The expressions are now being evaluated by https://github.com/phpmyadmin/simple-math

@ibennetch
Copy link
Member

Nice work @nijel!

@nijel
Copy link
Contributor Author

nijel commented Oct 13, 2016

Hmm, probably not so good as I've wrongly evaluated existing libraries and there is existing library which does serve our purpose well - https://packagist.org/packages/symfony/expression-language. It is more powerful (so it will be probably easier to adapt Advisor to it) and also performs better than SimpleMath (see phpmyadmin/motranslator#5 (comment)). So in the end SimpleMath will be abandoned and we will use ExpressionLanguage from Symfony.

nijel added a commit that referenced this issue Oct 13, 2016
See #6363, #11993 and phpmyadmin/motranslator#5

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this issue Oct 13, 2016
It changes API (to consistenly use camelCase) and removes use of eval().

See #6363, #11993 and phpmyadmin/motranslator#5

Signed-off-by: Michal Čihař <michal@cihar.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement A feature request for improving phpMyAdmin newbie
Projects
None yet
Development

No branches or pull requests

6 participants