thephpleague / plates

Native PHP template system
https://platesphp.com
MIT License
1.47k stars 180 forks source link

Less verbose section override? #48

Closed gmazzap closed 6 years ago

gmazzap commented 9 years ago

Currently, to define multiline default section content one needs an if / else block.

From your docs:

<?php if ($this->section('sidebar')) : ?>
        <?= $this->section('sidebar') ?>
<?php else: ?>
        <ul>
            <li><a href="/link">Example Link</a></li>
            <li><a href="/link">Example Link</a></li>
        </ul>
<?php endif ?>

In my fork, with this commit I added hasSection() template method that makes same thing doable using only an if statement, for less verbose and more readable templates.

Example:

<?php if ( ! $this->hasSection('sidebar')) : ?>
        <ul>
            <li><a href="/link">Example Link</a></li>
            <li><a href="/link">Example Link</a></li>
        </ul>
<?php endif ?>

If you like it I can write unit test and submit a PR.

jk3us commented 9 years ago

I think I'd argue against a method called hasSection() having the side-effect of echoing some content. But I wonder if something like this could work inside a layout:

<?php $this->startDefault('sidebar') ?>
<ul>
    <li><a href="/link">Example Link</a></li>
    <li><a href="/link">Example Link</a></li>
</ul>
<?php $this->stopDefault() ?>

The result would be that if sidebar is defined it gets used and this default section gets thrown away, otherwise the default section is used. It would use output buffering just like start() and stop().

gmazzap commented 9 years ago

@jk3us I'm agree that hasSection is not a great name, just not found a better one. Maybe printSection, doSection, useSection?

Your approach can work for sure, but:

I really think that if we find a good name my approach is ease to use and to implement and makes templates cleaner, otherwise the standard if / else approach is still the preferable way to go.

reinink commented 9 years ago

This is something I fiddled around with quite a bit when first writing the section logic. I really like how Twig does this. In the end I settled with what you see in the documentation because it's very clear what's going on.

I sort of agree with @jk3us that having something randomly printed is kind of weird, although it certainly does solve this issue, and in the context of templating this might be acceptable, especially of the function name indicates that printing will occur.

The challenge here is coming up with a good function name, and to make things more complicated I would want it to be one word, since all other Plates functions are one word. I'm thinking that the function output() might make the most sense:

<div id="sidebar">
    <?php if (!$this->output('content')): ?>
        <ul>
            <li><a href="/link">Example Link</a></li>
            <li><a href="/link">Example Link</a></li>
        </ul>
    <?php endif ?>
</div>

What do you think?

gmazzap commented 9 years ago

output() was the first name that I thought, the problem is that without any reference to sections it's hard to understand what's going on.

See your code, how can I understand that "content" is a section name and not a variable, a function or something else?

I understand the one word policy but in this case I think that with current Plates features is not possible to respect it and at same time keep all readable...

The problem with the if / else way is more evident when using nested sections, that means nested if / else blocks: templates become almost unreadable.

Moreover, due to #47 is not possible to use partials to "compose" complex layouts improving readability.

This is the main reason why I gave up using Plates and I went back to Twig for current projects, but I'm writing my library for native PHP templates, and in my library template inheritance is handled in the same way used by Twig.

I'm writing a different library instead of contribute to Plates because to obtain that I'm using a completely different approach that had required a too big rewrite for Plates...

ragboyjr commented 6 years ago

Closing in favor #169.