Do you have some feature in mind that you'd love to see in Couch? Let us know.
6 posts Page 1 of 1
Line 214 in tags.php:
from
Code: Select all
if( $page ){
to
Code: Select all
if( $page && $PAGE->tpl_is_clonable ){


Reason is - just one line for both clonable/regular templates
Code: Select all
<cms:get_custom_field var=s_fieldname masterpage=k_template_name page=k_page_name />

If template is non-clonable, that code won't get any output and some bigger construction is needed. So a little code update makes it easy for Couch to be 'smart' in this case and determine appropriate query.
For non-clonable templates, the right way of using this tag is simply this -
Code: Select all
<cms:get_custom_field var=s_fieldname masterpage=k_template_name />

i.e. no page name specified (as there are no pages in non-clonable templates).

If someone goes ahead and specifies a (obviously non-existent) page, strictly speaking that is a user error.
However, I agree, we can make the tag smart enough to cover this error.
KK wrote: However, I agree, we can make the tag smart enough to cover this error.

Would be great to see this in main codebase. Following construction would become redundant
Code: Select all
<cms:if k_template_is_clonable >
    <cms:set securefile = "<cms:get_custom_field s_fieldname masterpage=k_template_name page=k_page_name />" />
<cms:else /> 
    <cms:set securefile = "<cms:get_custom_field s_fieldname masterpage=k_template_name />" />
</cms:if>
Adding something related to the above -
if any time in future cms:get_custom_field supports id as a parameter on par with page, then
the same 'smart' check is very welcomed.
While I can totally see the utility of this check in the use-cases you mentioned, the solution you proposed i.e.
if( $page && $PAGE->tpl_is_clonable ){
- will, unfortunately, not suffice.

Reason being that the $PAGE object refers to the page being visited (i.e. as seen in the browser's URL) which may/can be different from the masterpage you are specifying with <cms:get_custom_field />.

Incidentally, if you are using <cms:get_custom_field /> right within a non-clonable template, then the following construct, though technically incorrect, will still work -
Code: Select all
<cms:get_custom_field s_fieldname masterpage=k_template_name page=k_page_name />

That is because on non-clobale templates, the k_page_name variable is not set at all and so the 'page' parameter above remains empty and is ignored.

Anyway, point I am trying to make is that to incorporate the check you are asking for will entail making an additional database query (to find if the masterpage is clonable or not) before formulating the real query (on line 222).

I am not sure if the use-case warrants making an expensive database access or whether the external <cms:if> you placed would suffice.
KK wrote: That is because on non-clobale templates, the k_page_name variable is not set at all and so the 'page' parameter above remains empty and is ignored.

This is not necessarily true, since k_page_name can still be available in the following sample environment:
Code: Select all
<cms:pages masterpage='clonable-template.php' limit='1'>
 
// k_page_name is set here

    <cms:pages masterpage='non-clonable-template.php' skip_custom_fields='1' >

     // and available here

        <cms:get_custom_field 'test' masterpage=k_template_name page=k_page_name />
    </cms:pages>
 
</cms:pages>


In conclusion, this seems to be a very specific use case, so a general solution would indeed require +1 db-query.
6 posts Page 1 of 1