PHP template style guide?

[jbidoret]

More a question than an issue…
Could we, especially within the admin / view / templates use this syntax:

<?php if($something): ?>
  <p><?= $something ?></p>
<?php endif ?>

instead of this one:

<?php if($something){
  echo "<p>$something</p>";
}?>

or even this one (that I find hard to catch with its closing brackets):

<?php if($something){ ?>
 <p><?= $something ?></p>
<?php } ?>

Apart of if/else/endif, I’ve got the same question with foreach / endforeach loops.

Within the templates, where a lot of HTML is echoed, I tend to find the first one cleaner, even slightly more verbose at some points. But I will stick to what you prefer.

[n-peugnet]

I completely agree. I think your proposition (1) is the best in templates. I discovered it later, so this is why almost all of these blocks use the style (3), which we kept using for consistency. IMO the style (2) should be completely avoided, and it should be very rare in the current templates.

I think that:

• If we want to switch from (3) to (1), we should do it across all templates.
• Based on current conventions, I would say that style (2) should be converted to style (3) if the conversion from (3) to (1) is not yet done.

[vincent-peugnet]

instead of this one:

<?php if($something){
  echo "<p>$something</p>";
}?>

Oh yeah, this one is not cool ! I agree that it should not be used.

Between your two proposals, I would say that the second one, with bracket, is the style already used every where. So maybe not mixing way to write this is a good idea. But I feel you when you say that it's not very visual. So, I don't know. Maybe there could be a standard for views different from the rest.

I'm curious about @n-peugnet 's opinion on this

[vincent-peugnet]

• If we want to switch from (3) to (1), we should do it across all templates.

Interesting but this seems not an easy refactor to me.

[jbidoret]

Among the app/view/templates, I think that I’ll go through every file, where HTML homogeneity will be the key.
I feel I could switch to (1) within every one, without too much problems, and without modifying any logic.
Do you think it could be OK to do the switch within this part of the CMS ?

[vincent-peugnet]

Do you think it could be OK to do the switch within this part of the CMS ?

I'm totally ok theoretically, but as I said, the refacture looks a bit rude in the non-templates parts. But maybe some magical regex could help 😋

I give it a try quickly to see how I feel about it.

[n-peugnet]

@jbidoret:

I feel I could switch to (1) within every one, without too much problems, and without modifying any logic.
Do you think it could be OK to do the switch within this part of the CMS ?

If done in a single commit as a standalone change, and it you do not change any indentation, it would be very easy to review, so I am in favour of it.

@vincent-peugnet:

the refacture looks a bit rude in the non-templates parts.

It should not be changed in non-template files.

[vincent-peugnet]

It should not be changed in non-template files.

aaahha okk, my bad, I misread what you wrote. Ok so it's chill for me !

And yess, one transition commit with this on templates would be lovely ❤️

[jbidoret]

Hmf. What a journey (there is quite a bunch of templates…).
The PR seems to have passed all the checks and my few non automated tests also!
#454

PS: I did two commits: one with simple switch and no re-indent, the other more complex, with re-indent.

[vincent-peugnet]

closed by #454