Skip to content
This repository has been archived by the owner on Nov 26, 2017. It is now read-only.

Use 'load_language' property as a flag to load the language files #1734

Merged
merged 3 commits into from Jan 25, 2013

Conversation

okonomiyaki3000
Copy link
Contributor

So that the constructor doesn't need to be overridden for this purpose.

It seems that almost all plugins override the constructor just to load language files. Now you will be able to simply set a property to true and they will be loaded automatically.

@eddieajau
Copy link
Contributor

Is there any reason why this couldn't be done via the $config array? Also, is it worth moving JPlugin to the legacy tree?

@okonomiyaki3000
Copy link
Contributor Author

In order for it to be done via the $config array, this option would need to be part of the plugin's params. I suppose it's possible to do it that way but params are typically meant to be set up by the user, right? Loading the language isn't something that should be at the user's discretion generally.

I'm not sure what you mean about the legacy tree. Would this change necessitate that?

@eddieajau
Copy link
Contributor

Loading the language isn't something that should be at the user's discretion generally.

I guess not, but then I'd argue it's at the discretion of the plugin developer. It honestly doesn't bother me, I just wouldn't include it because plugins don't usually have to load language files.

@okonomiyaki3000
Copy link
Contributor Author

Indeed, this leaves it at the discretion of the developer as it is now. The current situation is that, if a plugin needs language files, the developer typically overrides the constructor just to call loadLanguage. That means the parent constructor also needs to be called. So by using this property as a flag, we can save developers a minor hassle and reduce the overhead of calling additional functions.

It seems that about half of the plugins that ship with the CMS are overriding the constructor just for this purpose. Probably many third party plugins are as well.

@elinw
Copy link
Contributor

elinw commented Dec 3, 2012

Plugins need language files if they need configuration and also if they have to send any messages, and of course form plugins almost always will have labels and descriptions. Depending on the application they might have something like a success message on installation as well. So I think it's reasonable.

@eddieajau
Copy link
Contributor

plugins almost always will have labels and descriptions

Yes, but those language files are handled by com_plugins. Otherwise I just don't think it's a big deal for developers to have to load the language on demand. But like I said, it doesn't bother me.

I'll get you a determination on whether the maintainers want to move this package to the legacy tree (/libraries/legacy/plugin) as soon as I can.

* @var boolean
* @since 12.3
*/
protected $load_language = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd probably prefer to see this variable in camelCase $loadLanguage. We should only use underscore separators in things like column names.

@okonomiyaki3000
Copy link
Contributor Author

Isn't it kind of gross to use the same name for a property and a function?

@pasamio
Copy link
Contributor

pasamio commented Dec 5, 2012

Then make it autoloadLanguage which is slightly more descriptive for the property.

@eddieajau
Copy link
Contributor

Deal!

@okonomiyaki3000
Copy link
Contributor Author

Good thinking!

@elinw
Copy link
Contributor

elinw commented Dec 5, 2012

@eddieajau why do you think there will always be a com_plugin? But yeah you're right on that. I do think that the application probably has to implement a way to respond to triggers so you're totally right on the idea that it should handle the language for those kinds of things, but I will say that there will in general be language though so to +1 to this.

ianmacl pushed a commit that referenced this pull request Jan 25, 2013
Use 'load_language' property as a flag to load the language files
@ianmacl ianmacl merged commit fa0c073 into joomla:staging Jan 25, 2013
@elinw
Copy link
Contributor

elinw commented Jan 26, 2013

@okonomiyaki3000 Would you send this as a pull request to the cms?

@okonomiyaki3000 okonomiyaki3000 deleted the JPlugin-load_language branch January 26, 2013 14:32
@okonomiyaki3000
Copy link
Contributor Author

CMS pull request is here: joomla/joomla-cms#698

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

Successfully merging this pull request may close these issues.

None yet

5 participants