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

[#32929] com_plugins can't load a config.xml file #2660

Closed
wants to merge 23 commits into from
Closed

[#32929] com_plugins can't load a config.xml file #2660

wants to merge 23 commits into from

Conversation

andergmartins
Copy link
Contributor

This is the tracker link: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32929&start=0

The com_plugins component, doesn't recognise params form from a config.xml file, only if the config tag is inside the manifest file.

This is inconsistent since we can use a config.xml file in components and modules, why not in plugins?

…ates to look for the first valid manifest file as installation does.

- Fix com_modules models, to get the right XML file too.
# By Mark Dexter (6) and others
# Via Jean-Marie Simonet
* 'master' of https://github.com/joomla/joomla-cms: (22 commits)
  # Incomplete query in multilanguage status (synced with 2.5.x correction)
  # [#29104] Typo in 3.0.0 update script
  [#29102] Cannot create a new database during installation
  Remove templates/beez_20 folder
  wrong ini files in beez3 language folder
  Prepare for 3.0.0 beta1 release
  Follow up to postgres driver. Thanks Michael.
  Select old sample data in doInstall
  [#29025] Let the SEF plug-in add the canonical url to the head. Thanks Rouven.
  [#29089] new minimal sample data set. Thanks Elin.
  [#29087] Update Beez for Joomla 3
  [#29054] Update TinyMCE to 3.5.6
  [#29064] Improve the isis header
  [#28963] Add ordering to column titles where it was missing
  Fix some failing system tests for changes in master
  Fix a typo in c84ab42
  Fix codestyle from f6de2e7
  Fix query in com_menus
  [#29088] PostgreSQL driver for Joomla! CMS 3.0
  [#27979] PHP Memcached driver
  ...
# By Rouven Weßling (14) and others
# Via Rouven Weßling
* 'master' of https://github.com/joomla/joomla-cms: (30 commits)
  Update the code style ruleset for phpcs 1.3.6.
  Fix an error when generating cover coverage.
  Fix a typo from the last commit.
  Add unit tests for JAdministratorHelper.
  Fix an E_STRICT warning in com_banners. Thanks Elin.
  Remove unused variables found by phpmd.
  Make sure the build process works without error.
  Fix failing unit tests.
  Test the Feed package
  Add the following sniffs but only apply them to select folders: Generic.Files.LineLength PEAR.Functions.ValidDefaultValue Squiz.Operators.IncrementDecrementUsage Joomla.Classes.MethodScope Joomla.Commenting.FunctionComment Joomla.Commenting.SingleComment Joomla.Commenting.ClassComment Joomla.WhiteSpace.ConcatenationSpacing Joomla.ControlStructures.ControlSignature Joomla.ControlStructures.InlineControlStructure Joomla.ControlStructures.MultiLineCondition
  Code quality for beez3.
  Fix CRLF in beez3.
  Fix codestyle errors introduced in 69fdccd
  Beez3 fixing some files that were lost in various merges
  # [#29079] Move jquery chosen call from template to components. Thanks Viet Vu
  # [#29072] Contact view to use plain, tabbed, and slider outputs from Bootstrap JHTML library. Thanks Dave
  # [#29126] Admin Template: Accordion fieldsets for template and category options like com_menus. Thanks Reinhard
  # [#29099] Drop the unused #__update_categories table. Thanks Rouven
  # [#29062] Remove unnecessary table creations from testing sampledata. Thanks Rouven
  # [#29077] Restore popup button behavior, add button for new slider behavior, implement it. Thanks Michael
  ...
# By Jean-Marie Simonet (15) and others
# Via Jean-Marie Simonet (1) and others
* 'master' of https://github.com/joomla/joomla-cms: (56 commits)
  +$ Adding installation languages: bs-BA, hr-HR, lv-LV, mk-MK
  # [#28990] Unpublish default menu item gives a PHP exception. Thanks Roberto
  # [#29176] Call to undefined method JHtmlTabs::getJSObject() (Sync with platform). Thanks Elin
  # [#29091] *Language issues during installation: E-Mail always in English. Thanks Dennis
  [#29216] Clean up category lists. Thanks Elin.
  # +$ Adding installation languages: sr-RS, sr-YU, zh-TW
  [#28213] Page titles of categories and subcategories are unsuitable. Thanks Skunk Anan.
  [#29181] Make article info block semantic. Thanks Elin.
  [#29194] Add some alternatives to the Joomla Logo
  # [#28403] Validation error on com_finder. Thanks Emerson and Jason
  Comment typo
  missing changelog entry
  +# [#29180] Improvements/ clean up for beez 3. Thanks Elin
  +$ Adding installation languages: pl-PL, ta-IN, uk-UA
  # [#29201] Make it so submit button uses link color in protostar. Thanks Elin
  # Reverting #29141 as it creates unwanted pbs with dropbox while not solving fully the original issue
  # [#29208] com_contact categories list items count badge alignment. Thanks Jon
  # [#29207] Unpublished badge not using jtext. Thanks Elin
  #^ [#29160] Optimise JavaScript loading. Thanks Rouven
  # [#29179] SQL error in related items module. Thanks Mark
  ...
# By Jean-Marie Simonet (15) and others
# Via Jean-Marie Simonet (1) and infograf768 (1)
* 'master' of https://github.com/joomla/joomla-cms: (38 commits)
  # [#29281] Fix open div in com_content blog view (again). Thanks Kyle and Andy
  EOL errors
  Code quality.
  # [#29244] *Blocked/unblocked activated/not activated not displaying in user manager. Thanks Yannick
  Corrected some errors in lang files (EOL, etc.)
  # [#29286] *Template manager columns errors
  # [#29285] String Last Updated appears twice in frontend. Thanks Stella.
  $ Improving ta-IN installation ini
  #[#29274] Fix code style and unclear doc blocks from 29050. Thanks Michael
  # [#29050] Page Navigation in Extension Manager not working. Thanks yannick
  # [#29265] Turning on error reporting to Development throws errors on the front end. Thanks Mark Lee
  # Correcting lang string typo
  [#28992] Code cleanup for com_content. Thanks Viet Vu.
  [#29162] Need a Purge Cache Button in the Joomla! Update component. Thanks Janich Rasmussen.
  [#28900] Improvements to Schema package. Thanks Michael.
  [#29116] Parse error on initializing installation if server does not meet PHP requirements . Thanks Jean-Marie.
  [#28764] user notes category is not properly nested in the assets table. Thanks Elin.
  ckb-IQ forgotten
  # Correcting double translation
  +$ Adding installation languages: ar-AA, az-AZ, fi-FI
  ...
# By Jean-Marie Simonet (3) and others
# Via Jean-Marie Simonet
* 'master' of https://github.com/joomla/joomla-cms:
  changelog
  +$ Adding installation languages: ja-JP, ko-KR, nl-NL
  #$ Wrong lang constant and missing strings in hathor
  # [#29288] Joomla update doesnt purge correctly. Thanks Janich
  # [#29287] String Global Configuration appears as title of every component Options. Thanks Michael
Conflicts:
	administrator/components/com_modules/models/module.php
	administrator/components/com_modules/models/select.php
	administrator/components/com_plugins/models/plugin.php
… able to load a config.xml file, if existent
To be able to merge a pull request
@mbabker
Copy link
Contributor

mbabker commented Dec 14, 2013

Actually, only components can use the config.xml file (and have to). From what I can tell, components would actually be the extension with the inconsistent behavior; modules, plugins, and templates all have a single manifest file and config. I'm not sure I'd personally add in this mixed behavior for plugins.

@andergmartins
Copy link
Contributor Author

Hi,

Thanks, I'm just starting to contribute again, so I'm trying to understand how, or why, somethings are intended to be, or have different behaviour. I just would like to understand…

As components, plugins, templates and models are all extensions, I can't see why they have different rules for the same "information", the config data. It is nothing than a form file (the config.xml), so why should it be inside the manifest file for some types of extensions and not for the other? Shouldn't we always work to add consistency? So, if the use, meaning and behaviour are the same (the config descriptions), why force different ways to specify that?

The main reason to have a manifest file is for installation purposes and to describe the extension's meta data. So why add a kind of information that will not be used during the installation process, or even will be not cached with the rest of the manifest data? So if we tell that the manifest data is cached, but the config data is not, we have inconsistency since that tags are inside the manifest file…

And it is not forcing to use a separated config file, it only allows to use that, if I want… Less files in the extension folder is clean, nice, but using different files to store different kinds of information, sounds more organised, for me… For example, I don't think that is a good practice to write more than one class on a unique PHP file, so for the same reason, we shouldn't mix installation data with a form data (the config)

@andergmartins
Copy link
Contributor Author

You can see on this doc page: http://docs.joomla.org/Manifest_files on the "Configuration" subtitle:

...The element, a child of the root, describes the configuration options for the extension. If applicable, the options will be shown by the appropriate Manager (Plugin Manager, Module Manager or Template Manager). Configuration options can also be defined in a separate file named config.xml. Its root element should be …

That documentation is not only for components, it only warnings about that is the standard behaviour for components, but the idea of to use a different file for configuration is clean on that paragraph…

Actually I still couldn't test it for modules or templates… But I think that all extensions should support, or even force, to use a separated config file, like components...

@Bakual
Copy link
Contributor

Bakual commented Dec 15, 2013

@andergmartins The main reason probably is that while they are all extensions, they are not the same and it's different components dealing with its settings in each case.
Modules and plugins usually have only a few settings and it works well to have those settings in the manifest file. Components tend to have much more options, so it made sense to have them all in a separate file.

I agree with Michael here that if we want to have consistency, we would have to change the component behavior :)
But then I'd rather change the doc and say that the components behaves differently here.

I would not add a second alternative option to look for config stuff. It only makes code more complicated than it already is and doesn't add any real value.

@andergmartins
Copy link
Contributor Author

Ok, got it, thanks. But I still think that one "if" statement is not actually a complicated code… I agree with you about components are usually more complex than modules or plugins, but some modules are really full of params, more than many components… so if Joomla guys decided to change this behaviour for component some time ago, I think it should be replicated to the other extensions… I know that each extension is handled by different components, but since all that components are native, from the same framework, all of them should have the same rules for the same kind of information… so instead of add a second alternative, lets change that to mirror the components rules for config, using a separated file…

Consistency, for itself is not a real value? I can make the changes and update this pull request, for the components, modules, plugins, according what will be better for Joomla… if you agree.

@Bakual
Copy link
Contributor

Bakual commented Dec 15, 2013

I'd say update the PR to offer the same function across components, modules, plugins and templates (so it's really consistent).
If enough testers test it (usually two), it will be considered for inclusion.

@andergmartins
Copy link
Contributor Author

Right, working on that ;)

@betweenbrain
Copy link
Contributor

I can see the value of adding this behavior, not only for the sake of consistency, but especially if it is not mandatory. I have seen use cases where a plugin or module could benefit from this separation of code.

@b2z
Copy link
Member

b2z commented Jul 27, 2014

@test successful

@Kubik-Rubik
Copy link
Member

This PR can only be accepted if you provide the same consistent behavior for all types of extensions (missing here: modules and templates). Please update the PR, so we can take another look on it.

Thank you!

@andergmartins
Copy link
Contributor Author

Ok, thanks. I will work on that.

@Bakual
Copy link
Contributor

Bakual commented Jul 28, 2014

Also, please create a new PullRequest targeted at the staging branch then and close this one. Thanks.

@andergmartins andergmartins changed the title com_plugins can't load a config.xml file [#32929] com_plugins can't load a config.xml file Aug 21, 2014
@jissues-bot jissues-bot changed the title [#32929] com_plugins can't load a config.xml file com_plugins can't load a config.xml file Oct 17, 2014
@brianteeman
Copy link
Contributor

@andergmartins It has been 4 months since the last comment from you. Are you still planning to contribute code for this or shall I close this issue

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/2660.

@andergmartins
Copy link
Contributor Author

Hi @brianteeman, yes, I will contribute, please keep it open.

Thanks

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/2660.

@andergmartins andergmartins changed the title com_plugins can't load a config.xml file [#32929] com_plugins can't load a config.xml file Dec 7, 2014
@andergmartins
Copy link
Contributor Author

I'm working on these fixes, and closing this PR to create a new one...

@andergmartins
Copy link
Contributor Author

Just created a new PR, as requested: http://issues.joomla.org/tracker/joomla-cms/5342

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/2660.

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.

None yet

8 participants