Skip to content

Use motranslator for translating #35

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

Closed
wants to merge 21 commits into from
Closed

Use motranslator for translating #35

wants to merge 21 commits into from

Conversation

nijel
Copy link
Contributor

@nijel nijel commented Feb 24, 2016

This makes it work transparently with or without phpMyAdmin.

See also phpmyadmin/phpmyadmin#12023

Signed-off-by: Michal Čihař michal@cihar.com

@codecov-io
Copy link

Current coverage is 99.17%

Merging #35 into master will increase coverage by +0.01% as of 10ac2b3

@@            master     #35   diff @@
======================================
  Files           48      49     +1
  Stmts         3481    3497    +16
  Branches         0       0       
  Methods        158     161     +3
======================================
+ Hit           3452    3468    +16
  Partial          0       0       
  Missed          29      29       

Review entire Coverage Diff as of 10ac2b3

Powered by Codecov. Updated on successful CI builds.

*
* @return Translator object
*/
public static function getInstance()

Choose a reason for hiding this comment

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

Can't we make gettext a static method and use Translator::gettext($msg) instead of Translator::getInstance()->gettext($msg). Looks much cleaner and easier to use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed that would be doable as well with lazy initialization of the translator (we don't need it in most cases, it's used only for errors).

@nijel nijel changed the title Use motranslator to provide __ function Use motranslator for translating Mar 2, 2016
@nijel
Copy link
Contributor Author

nijel commented Mar 2, 2016

Thinking more about this I'm not sure if it's good idea to include this in releases used in phpMyAdmin 4.6.x as this will bring two gettext implementation into phpMyAdmin codebase....

@nisargjhaveri
Copy link

If we want to think really 'modular', it shouldn't matter :P . But yes, makes sense.

How are we actually managing releases of SQL parser?

@nijel
Copy link
Contributor Author

nijel commented Mar 5, 2016

It won't matter for 4.7, but for 4.6 I'm not sure if switching phpmyadmin to motranslator is reasonable...

@nisargjhaveri
Copy link

Agreed.

@nijel nijel self-assigned this Sep 13, 2016
nijel added 21 commits December 21, 2016 15:02
This makes it work transparently with or without phpMyAdmin.

Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
[CI skip]

Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
Otherwise it's for user hard to diagnose what went wrong.

Signed-off-by: Michal Čihař <michal@cihar.com>
[CI skip]

Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
[CI skip]

Signed-off-by: Michal Čihař <michal@cihar.com>
Signed-off-by: Michal Čihař <michal@cihar.com>
@nijel
Copy link
Contributor Author

nijel commented Dec 21, 2016

Superseded by #106

@nijel nijel closed this Dec 21, 2016
@nijel nijel deleted the motranslator branch December 21, 2016 14:27
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.

3 participants