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

Add i18n translate alias function to bootstrap #39

Merged
merged 1 commit into from
Aug 12, 2014
Merged

Add i18n translate alias function to bootstrap #39

merged 1 commit into from
Aug 12, 2014

Conversation

lenton
Copy link

@lenton lenton commented Aug 7, 2014

Related to kohana/core#526

When implementing I realised that I18n::lang() had to be set for the function to be useful, so in the end it did have to be placed after Kohana::modules().

@enov
Copy link
Contributor

enov commented Aug 8, 2014

I18n::lang() has already a default:

https://github.com/kohana/core/blob/3.4/develop/classes/Kohana/I18n.php#L26

So I think it will work anyways if you have it before Kohana::modules(), no?

If the user will change the language in the bootstrap, the function will emit English (the default) translations only when it is used in init.php of modules. It will continue emitting the desired translation messages afterwards. Also, the user can extend I18n and change that default through the CFS.

If it is possible to put it before the modules, it would be better. Personally, I prefer the inconsistency of the languages over not being defined. Other opinions are welcome.

@lenton
Copy link
Author

lenton commented Aug 8, 2014

The __() function doesn't need to be defined before Kohana::modules() in a hacky way anymore, this solves the problem: kohana/core#530.

@@ -87,6 +87,15 @@
*/
I18n::lang('en-us');

// Define i18n translate alias function
if ( ! function_exists('__'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to wrap it with function_exists? I know it does not hurt, but it's redundant since bootstraping is done only once.

Copy link
Author

Choose a reason for hiding this comment

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

I left it in because the index.php doesn't explicitly say that the bootstrap should only be included once https://github.com/kohana/kohana/blob/3.4/develop/index.php#L103 and there are existing checks for other globals https://github.com/kohana/kohana/blob/3.4/develop/index.php#L89.

@acoulton
Copy link
Member

@lenton @enov Is this OK to merge? It looks OK to me - the 3.4 build fails without it.

@lenton
Copy link
Author

lenton commented Aug 12, 2014

@acoulton Yep, this also needs to be merged kohana/core#530 otherwise you'd get some more errors afterwards.

@acoulton acoulton merged commit c65a84a into kohana:3.4/develop Aug 12, 2014
acoulton added a commit that referenced this pull request Aug 12, 2014
Closes #39

Conflicts:
	application/bootstrap.php
@acoulton
Copy link
Member

@lenton thanks - actually the build runs OK (apart from issues related to the Kohana::modules() paths) without kohana/core#530.

@enov enov mentioned this pull request Mar 26, 2015
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