User tests: Successful: Unsuccessful:
Our controller can only fetch model and view by plural name, I add singular name style to support new naming style.
Labels |
This change not only for new MVC, if people want to use autoloading in legacy component, singular name can help this working well since table and form has already support singular name.
https://github.com/joomla/joomla-cms/blob/staging/libraries/legacy/model/legacy.php#L265
I think it will helpful when some component in a working process when they are changing to new style.
Labels |
Added:
?
|
I agree with @asika32764's change.
You shoud notice that in the old MVC classes are already using singular names (which can be considered a bug in the whay that the legacy MVC works). If you want to have autoloading working out of the box in your component you can directly name models
folder to model
but for BC we have to keep models
working.
That's why we need both working
I double checked it and if it's supposed to be autoloading it shouldn't be added manually like this PR does.
It should be enough with adding to your front controller. Example for content:
// Register component prefix
JLoader::registerPrefix('Content', __DIR__);
The real fix would be to replace the current createModel
method with:
/**
* Method to load and return a model object.
*
* @param string $name The name of the model.
* @param string $prefix Optional model prefix.
* @param array $config Configuration array for the model. Optional.
*
* @return mixed Model object on success; otherwise null failure.
*
* @since 12.2
* @note Replaces _createModel.
*/
protected function createModel($name, $prefix = '', $config = array())
{
// Clean the model name
$modelName = preg_replace('/[^A-Z0-9_]/i', '', $name);
$classPrefix = preg_replace('/[^A-Z0-9_]/i', '', $prefix);
$className = ucfirst($classPrefix) . ucfirst(strtolower($modelName));
if (class_exists($className))
{
return new $className($config);
}
$result = JModelLegacy::getInstance($modelName, $classPrefix, $config);
return $result;
}
That will really make autoloading work.
@asika32764 can you double check if that fixes the issue and update your PR if you agree? Thanks!!
Thank you, I will check it when I have time.
I think we are discussing without a base position, me and @phproberto focus on autoloading, and we think autoloading is not equal to new MVC, so we trying to make legacy MVC work in autoloading with singular name.
So, this change depend on maintainer think autoloading & singular name is same as new MVC or not.
@asika32764 I would be fine with the approach of Roberto. Follow his lead
Title |
|
Title |
|
Thanks @phproberto , this code working well.
Actually I use this way at the beginning, but in this way, view have to build class name as ContentViewArticleHtml
, I really don't want to mix new MVC's style in ControllerLegacy, so I choosing to use setPath()
instead build this class name in the createView()
.
So, what everyone think about how to approach the view?
Labels |
Removed:
?
|
Labels |
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
Title |
|
Title |
|
@asika32764 Can you please provide step-by-step test instructions so we know how to test your pull request? Thank you. If no test instructions are provided within 4 weeks, this PR will be closed. Thank you for your contribution.
Status | Pending | ⇒ | Information Required |
Labels |
Added:
?
|
Title |
|
Labels |
Added:
?
|
Title |
|
Since no information has been provided I am going to close this issue. Feel free to re-open when you are ready and the issue still exists.
Status | Information Required | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-06-12 22:12:09 |
Closed_By | ⇒ | roland-d |
I don't see the point in this at all. The singular name style is to allow the use of the JModel Class. The plural names are for the legacy classes. I don't think we should be letting people use the legacy classes in the singular view name styles