I'm not sure what the purpose is. It's already possible to use custom fields by providing the addfieldpath to the XML.
If I understand that correct, you just want to automate this so you don't have to define the path yourself?

@Bakual three things are done with this simple line of code:
1. Code doesn't need to be hard coded in xml's files
2. We get a unified way to do things
3. It makes things easier for dev's

Code doesn't need to be hard coded in xml's files

Hmm, so instead of "hardcoding" the path it in the XML, we hardcode it in the model.

We get a unified way to do things

Actually, you introduce a new way to do things. Now one could either use the predefined path or specify an own in the XML. But only in plugins and templates. Components and modules would need additional PRs to be in line with this change, right?

It makes things easier for dev's

A tiny bit, yes. However I have to say it's very simple to add this attribute to the XML. This option needs to stay anyway, because extensions may have their fields in a library or other places.

I'm not sure if I like it or not. It's only a tiny gain imho at the cost of adding a tiny bit of additional complexity to the system. Let's see what others think.

Can you maybe adjust the title and description of the PR? Currently it's misleading as it has nothing to do with allowing custom fields for plugins (or templates). That is already possible without any issues by providing the addfieldpath attribute in the XML.

@Bakual

Hmm, so instead of "hardcoding" the path it in the XML, we hardcode it in the model.

I put it as a new more standardized workflow, but that maybe only my opinion

Don't understand what you meant to say. Do see that you don't want to use JPATH_COMPONENT_ADMINISTRATOR for some reason. From a programming point of view consider your code wrong. The constant JPATH_COMPONENT_ADMINISTRATOR is declared a few lines up, you have a chance to use it, but choose to recalculate it. Apart from the fact it is inconsistent it is bad programming!

No, use JPATH_COMPONENT_ADMINISTRATOR in both if and addFieldPath instead of calculating it twice!

@sovainfo Honestly is late, you are right I PR the change

tried to do that as well, can't pr your pr!

@sovainfo Did I get it right now?

Nice change

While playing around I found that components already support that
See https://github.com/joomla/joomla-cms/blob/staging/libraries/legacy/model/form.php#L172

For plugins, modules and templates this doesn't work because the executed component is com_plugins, com_modules and com_templates.

Thus I would suggest to just add the path similar as done in JModelForm without checking the presence of the folder first. That is taken care later on when Joomla tries to load the field.

@Bakual Will do that! Thanks
The funny part on this PR is that the first commit was just that line ????

Some documentation updates needed if this gets approved.
These are the ones I found:
http://docs.joomla.org/Form_field
http://docs.joomla.org/Creating_a_custom_form_field_type
http://docs.joomla.org/Creating_a_modal_form_field

Disagree on removing the path existence check. Considering the amount of processing don't think it is a good idea to introduce a lot of paths that don't exist. It does take memory and cpu, why waste it?
My definition of efficient and DRY is different, I guess! It doesn't make sense to me to tell looking at places that don't even exists.
To be consistent those checks should be done for the component as well.

Disagree on removing the path existence check. Considering the amount of processing don't think it is a good idea to introduce a lot of paths that don't exist.

I'm sure you will not be able to measure the difference. It's only a single path added. We also do a lot more such path checks for our override system, class autoloading and many more. It's not something which uses lot of ressources to process.

On the other hand, adding is_dir checks makes the code more complex and less readable.

@Bakual @sovainfo So what’s the decision here, besides the admin/front end path definition?

Can't take those arguments seriously. But have no interest to continue that discussion with someone I consider unreasonable.

@dgt41 Apologize for assisting you, looks like my input is not appreciated and will cause it not to be implemented. Sorry, maybe there are others that do understand the importance.

@sovainfo On the above argument I think what @Bakual says, and he is correct, is that if we do a path existence check here we are gonna repeat the procedure when we actually gonna add the fields. On the other hand the majority of modules, plugins and templates don’t use custom fields, so maybe it will be beneficial not to introduce them in the first place, and thus save some resources. Both ways are fine for me, because in either of them the harm of not doing it the other way is really marginal.

Either way will work.
Adding is_dir checks will make the code a tiny bit faster since you only check the presence of the path once instead for each field. However that is probably only measureable if you are talking about some hundred fields. In practice that will be seldom the case. Also, extension which have so many fields, most likely have at least one custom field and thus already have this path added anyway.

On the other hand it makes the code harder to read if you have

if (is_dir(JPATH_COMPONENT . '/models/forms'))
{
    JForm::addFormPath(JPATH_COMPONENT . '/models/forms');
}

if (is_dir(JPATH_COMPONENT . '/models/fields'))
{
    JForm::addFieldPath(JPATH_COMPONENT . '/models/fields'); 
}

if (is_dir(JPATH_COMPONENT . '/models/form'))
{
    JForm::addFormPath(JPATH_COMPONENT . '/model/form'); 
}

if (is_dir(JPATH_COMPONENT . '/models/field'))
{
    JForm::addFieldPath(JPATH_COMPONENT . '/model/field'); 
}

instead of

JForm::addFormPath(JPATH_COMPONENT . '/models/forms'); 
JForm::addFieldPath(JPATH_COMPONENT . '/models/fields'); 
JForm::addFormPath(JPATH_COMPONENT . '/model/form'); 
JForm::addFieldPath(JPATH_COMPONENT . '/model/field'); 

On a sidenote: Performance is important for pages which are shown to visitors on a regular basis. Performance doesn't really matter that much if we're talking about administrator views like we're talking here. It's not like you're going to request a module edit view several times per second.

For me, code readabilty wins hands down in this case. But it may be a personal preference. I don't care that much honestly.

I guess I’ll better leave it as is right now, till we get some more input. And if it gets some attention and successful tests I will also provide the documentation changes needed as well.

Should I apply only this PR to test it?

This comment was created with the J!Tracker Application at http://issues.joomla.org/.

@b2z For the plugin and module it will be easier if you apply also patch #4240 because there is some custom field there so you can use it. For the templates part follow the instructions at #4295.

I will break this to three PRs with some dummy template, module, plugin to make testing easier

@dgt41 @bakual @sovainfo Why don't we make the is_dir implicit in JFormHelper::addPath() https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/form/helper.php#L307 and prevent adding of an invalid path in the first place.
Why are we discussing about the repetitive checks, be it anywhere!

Sorry replied to wrong post

@izharaazmi you got a point here!

@izharaazmi Sounds good to me in this case. Normally not in favor of postponing things but it looks like that is the right place to avoid adding non-existing folders to the path.
That might also be the correct place to translate /administrator. Although I would prefer the xml to be extended with an attribute for application and extending the path in case of administrator. The translation would be required still for dealing with old xml though

This should definitely be applied to J3, finally dealing with the inconsistency of suggesting the folder can be named different from administrator and hardcoding it in the rest of the software.