Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Adding Translation to template (WIP) #112

Closed
wants to merge 9 commits into from

Conversation

tony199555
Copy link
Contributor

Using the method provided by : https://github.com/smarty-gettext/smarty-gettext

Strings that are translated are not showing on the new page. Maybe somewhere I missed that should be included.

@tony199555
Copy link
Contributor Author

tony199555 commented May 26, 2017

PR for #111

Copy link
Owner

@Rudloff Rudloff left a comment

Choose a reason for hiding this comment

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

Thanks for your work on translating Alltube 😄
I added comments on a few things that might prevent gettext from working correctly.

$language = "zh_CN";
putenv("LANG=".$language);
setlocale(LC_ALL, $language);
?>
Copy link
Owner

Choose a reason for hiding this comment

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

PHP code in template files won't be executed.
You need to put this in index.php for example.

@@ -0,0 +1,135 @@
msgid ""
Copy link
Owner

Choose a reason for hiding this comment

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

If the gettext domain you are using is AllTube this file should probably be named AllTube.po.

composer.json Outdated
@@ -13,7 +13,8 @@
"ptachoire/process-builder-chain": "~1.2.0",
"guzzlehttp/guzzle": "~6.2.0",
"rudloff/rtmpdump-bin": "~2.3",
"aura/session": "~2.1.0"
"aura/session": "~2.1.0",
"smarty-gettext/smarty-gettext": "~1.2.0"
Copy link
Owner

Choose a reason for hiding this comment

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

You should use smarty-gettext 1.5.1.
1.2.0 does not work correctly with the way Smarty is set up in Alltube.

Copy link
Owner

@Rudloff Rudloff left a comment

Choose a reason for hiding this comment

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

I added a small change I had to make in order to make it work on my system.
Other than that, it works correctly. Thanks!

index.php Outdated

$language = "zh_CN";
putenv("LANG=".$language);
setlocale(LC_ALL, $language);
Copy link
Owner

Choose a reason for hiding this comment

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

On some systems, the locale is called zh_CN.utf8, so you should do something like this:

setlocale(LC_ALL, [$language, $language.'.utf8']);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not able to get the result even changing the setlocale to utf8, which I am really have no idea why. Do you have a sample site that you have done successfully? Also if you don't mind can you push the change to a different branch so I can look at it.

@codecov-io
Copy link

codecov-io commented May 27, 2017

Codecov Report

Merging #112 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #112   +/-   ##
=========================================
  Coverage     99.72%   99.72%           
  Complexity       97       97           
=========================================
  Files             5        5           
  Lines           363      363           
=========================================
  Hits            362      362           
  Misses            1        1

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 fed425f...b4dc208. Read the comment docs.

@tony199555
Copy link
Contributor Author

OK, I see. But I cannot get it working. If possible it would be great that you put your changed code into a different branch (perhaps feature/translate) that I could take a look at. Along with that, can you make a option to choose different language?

@Rudloff
Copy link
Owner

Rudloff commented May 27, 2017

The only change I was talking about was the .utf8 thing. Now that you added it, your code works out of the box on my computer.

Note that you need to have the zh_CN locale installed/enabled on your server for gettext to work.
On Linux, you can check the installed locales with locale -a. If for some reason it is called differently on your OS, you will need to add the exact name to setlocale().

@Rudloff
Copy link
Owner

Rudloff commented May 27, 2017

Along with that, can you make a option to choose different language?

Yes, it would totally make sense to add this to config.yml. Do you want to work on this yourself?
If you don't feel comfortable doing it, I will merge your PR then add this option.

@tony199555
Copy link
Contributor Author

Note that you need to have the zh_CN locale installed/enabled on your server for gettext to work.
On Linux, you can check the installed locales with locale -a. If for some reason it is called differently on your OS, you will need to add the exact name to setlocale().

That is exactly why I don't see it. I though it will render on the client instead on the server, but since it is a template it should be render on server.

Yes, it would totally make sense to add this to config.yml. Do you want to work on this yourself?
If you don't feel comfortable doing it, I will merge your PR then add this option.

What I mean is a switch on the web page, somewhere on the top right (or left), append url with locale=xx_XX (or cookie if you want to), instead of hardwired. At that case, I am not familiar with the implementation.

@Rudloff
Copy link
Owner

Rudloff commented May 29, 2017

I have merged your work into the feature/gettext branch.
I will work on a language switcher for the next release.

@tony199555
Copy link
Contributor Author

tony199555 commented May 30, 2017

I am actually doing the options to switch language, and I am about half way there....although the method might get ugly. Yet I am not using cookie, so it will needed to switch every time. I will look into it and see how to do it.

P.S. I just saw your work and I am going give up what I am going to do. But my thought so far:

<?php
$trans_dir = '../i18n/Translations/*';
$globtrans = glob($trans_dir, GLOB_ONLYDIR);
foreach ($globtrans as $dlang) 
		{
            $lang[] = $dlang;
          //get array from glob, which find out what locale is available
}

foreach ($lang as $langpath) {
            $langxpath[] = basename($langpath);
           //get rid of path
}

foreach ($langxpath as $langlocale){
	   $displayname[] = locale_get_display_name($langlocale, $langlocale);
                   /**show the locale in its locale name, i.e. 
                     français (France)
                     中文(中国)
                     中文(台灣)
                   **/
}
?>

This is a good practice for me as I am learning php. 😆

@Rudloff
Copy link
Owner

Rudloff commented May 30, 2017

Sorry, as you've seen, I started working on this myself.
But I'll take your ideas into account. (I didn't know about locale_get_display_name(), it seems pretty useful here).

Also, I'm closing this pull request since I merged your code manually.
Thanks again for your contribution! Please feel free to open a new issue if you have new enhancements to propose.

@Rudloff Rudloff closed this May 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants