Tracker item 33726

Ah well, there's another thingy beside the code logic...
We usually use colon notation for control structures in PHP template files like this:

<?php if (condition) : ?>
    // Do something
<?php else : ?>
    // Do something else
<?php endif; ?>

There is no coding standard that would actually enforce this rule, but since the rest of the file already has this notation, I think this should be changed.

Thanks for your patience

I understand the importance of style and am happy to make any changes
but I'm afraid I'm not sure what you are asking.

I assume that you are talking about the code block:

<?php
$xmldata = new JRegistry;
if (!$item->xmldata){
$xmldata->set("version", "--");
$xmldata->set("creationDate", "--");
}else{
$xmldata->loadObject($item->xmldata);
if (!$xmldata->get('version')){
$xmldata->set("version", '--');
}
if (!$xmldata->get('creationDate')){
$xmldata->set("creationDate", '--');
}
}
?>

Are you suggesting that I change it like this?

<?php $xmldata = new JRegistry; ?>
<?php if (!$item->xmldata) : ?>
<?php $xmldata->set("version", "--"); ?>
<?php $xmldata->set("creationDate", "--"); ?>
<?php else : ?>
<?php $xmldata->loadObject($item->xmldata); ?>
<?php if (!$xmldata->get('version')) : ?>
<?php $xmldata->set("version", '--'); ?>
<?php endif; ?>
<?php if (!$xmldata->get('creationDate')) : ?>
<?php $xmldata->set("creationDate", '--'); ?>
<?php endif; ?>
<?php endif; ?>

If so, I'll make the change. If not, perhaps you could enlighten me :)

Bill

On 5/12/2014 11:58 PM, Nikolai Plath wrote:

Ah well, there's another thingy beside the code logic...
We usually use colon notation for control structures in PHP template
files like this:

|<?php if (condition) : ?>
// Do something
<?php else : ?>
// Do something else
<?php endif; ?>
|

There is no coding standard that would actually enforce this rule, but
since the rest of the file already has this notation, I think this
should be changed.

Thanks for your patience

—
Reply to this email directly or view it on GitHub
#3589 (comment).

No virus found in this message.
Checked by AVG - www.avg.com http://www.avg.com
Version: 2014.0.4577 / Virus Database: 3931/7483 - Release Date: 05/12/14

Yes that would be it, but I think we all agree that it looks like hell...

The best solution would be not to mix that much PHP logic in template files.

The second best solution, IMHO, would be to use just one pair of PHP tags to surround the whole block

<?php
       $xmldata = new JRegistry;
       if (!$item->xmldata) :
            $xmldata->set("version", "--");
            $xmldata->set("creationDate", "--");
       else :
            $xmldata->loadObject($item->xmldata);
            if (!$xmldata->get('version')) :
                 $xmldata->set("version", '--');
            endif;

            if (!$xmldata->get('creationDate')):
                 $xmldata->set("creationDate", '--');
            endif;
        endif;
?>

What do you think?

Well, your second-best solution looks like the code that I've already
committed. That would be ok with me, I'm just trying to protect other
people who have migrated sites from 1.5 to 1.6 to 1.7 to 2.5 from the
frustration that I experienced when the template manager failed.

The alternative approach would be to put the code into
administrator/components/com_templates/models/templates.php around line
65. That would solve the problem for all templates but it's much harder
to know what the side effects might be.

Bill Bean

On 5/13/2014 2:03 PM, Nikolai Plath wrote:

Yes that would be it, but I think we all agree that it looks like hell...

The best solution would be not to mix that much PHP logic in template
files.

The second best solution, IMHO, would be to use just one pair of PHP
tags to surround the whole block

|<?php
$xmldata = new JRegistry;
if (!$item->xmldata) :
$xmldata->set("version", "--");
$xmldata->set("creationDate", "--");
else :
$xmldata->loadObject($item->xmldata);
if (!$xmldata->get('version')) :
$xmldata->set("version", '--');
endif;

         if (!$xmldata->get('creationDate')):
              $xmldata->set("creationDate", '--');
         endif;
     endif;

?>
|

What do you think?

—

Reply to this email directly or view it on GitHub
#3589 (comment).

No virus found in this message.
Checked by AVG - www.avg.com http://www.avg.com
Version: 2014.0.4577 / Virus Database: 3950/7486 - Release Date: 05/13/14

@whbean same issue is here:
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_templates/views/template/tmpl/default_description.php

If you access the Template Manager: Customise Template -> Template Description

Yes, that is much the same problem, although in your case it is the
description field, which was not involved in the one that I found. We
could fix this by surrounding the reference to xmldata->description with
an if statement. Note that modifying
administrator/components/com_templates/models/templates.php around line
65 would not fix this problem since the description file bypasses the
code in TemplatesModelTemplates::getItems.

I am unclear on where this process stands. Do I need to do something more?

Bill Bean

On 5/16/2014 2:29 PM, zero-24 wrote:

@whbean https://github.com/whbean same issue is here:
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_templates/views/template/tmpl/default_description.php

If you access the Template Manager: Customise Template -> Template
Description

—
Reply to this email directly or view it on GitHub
#3589 (comment).

No virus found in this message.
Checked by AVG - www.avg.com http://www.avg.com
Version: 2014.0.4577 / Virus Database: 3950/7504 - Release Date: 05/16/14

Thank you for your contribution, @whbean. I've just tested it in the current staging version and no errors occured. The fields for the values were just blank. Since this is not a bug, I'll close this PR now. Feel free to do a new PR if you still have a problem!