Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML trapped inside echo statements #46

Open
halfer opened this issue Oct 21, 2014 · 0 comments
Open

HTML trapped inside echo statements #46

halfer opened this issue Oct 21, 2014 · 0 comments

Comments

@halfer
Copy link

halfer commented Oct 21, 2014

I suggest the following code (taken from here):

<html>
    <head>
        <title>Suggestotron</title>
    </head>
    <body>
        <?php
        foreach ($topics as $topic) {
            echo "<h3>" .$topic['title']. " (ID: " .$topic['id']. ")</h3>";
            echo "<p>";
            echo nl2br($topic['description']);
            echo "</p>";
            echo "<p>";
            echo "<a href='/edit.php?id=" .$topic['id']. "'>Edit</a>";
            echo " | ";
            echo "<a href='/delete.php?id=" .$topic['id']. "'>Delete</a>";
            echo "</p>";
        }
        ?>
    </body>
</html>

becomes like so:

<html>
    <head>
        <title>Suggestotron</title>
    </head>
    <body>
        <?php foreach ($topics as $topic): ?>
            <h3>
                <?php echo $topic['title'] ?> (ID: <?php echo $topic['id'] ?>)
            </h3>
            <p>
                <?php echo echo nl2br($topic['description']) ?>
            </p>
            <p>
                <a href="/edit.php?id=<?php echo $topic['id'] ?>">Edit</a>
                |
                <a href="/delete.php?id=<?php echo $topic['id'] ?>">Edit</a>
            </p>
        <?php endforeach ?>
    </body>
</html>

Reasoning: HTML should not be trapped inside echo statements. It prevents modern editors from syntax-checking the HTML or colourising it. Also, there's less need for concatenation, so it looks nicer.

Incidentally, I've swapped the brace-style foreach for a colon-style. There's a rule of thumb that the former belongs in code files and the colon-style is preferred in template files, because the ending is much more explicit. Not essential for this ticket but nice to add, IMO.

I expect there are others too, but it's worth raising a ticket to see if this is in keeping with house style first. Of course, htmlspecialchars() is required too, but that's another ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant