Skip to content

Dedent: fixed indentation preservation for paired tags inside HTML#414

Open
davidkrmela wants to merge 5 commits intonette:masterfrom
davidkrmela:dedent-preserve-structural-indentation
Open

Dedent: fixed indentation preservation for paired tags inside HTML#414
davidkrmela wants to merge 5 commits intonette:masterfrom
davidkrmela:dedent-preserve-structural-indentation

Conversation

@davidkrmela
Copy link
Copy Markdown
Contributor

  • bug fix
  • BC break? no

When paired tags like {foreach} or {if} are indented inside HTML markup, dedent now preserves the tag's structural indentation level instead of stripping it completely.

As described in the blog article, this template:

<ul>
    {foreach $items as $item}
        <li>{$item}</li>
    {/foreach}
</ul>

now correctly generates:

<ul>
    <li>...</li>
    <li>...</li>
</ul>

Previously, the indentation was stripped completely, producing <li> at column 0.

Also works for nested tags ({foreach} + {if} inside HTML).

Copilot AI review requested due to automatic review settings April 4, 2026 11:15
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Latte’s dedent behavior so that when paired tags (eg {foreach}, {if}) are indented inside HTML markup, the rendered output keeps the HTML structural indentation instead of stripping it to column 0.

Changes:

  • Added new dedent regression tests for {foreach} and nested {foreach}/{if} inside HTML.
  • Updated TemplateParser::applyDedent() to preserve indentation up to the tag’s structural indentation level when removing “excess” indentation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
tests/common/dedent.phpt Adds regression tests verifying dedent keeps HTML indentation for paired tags.
src/Latte/Compiler/TemplateParser.php Adjusts dedent logic to preserve indentation up to a computed tag indentation level.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

$atLineStart = true;
$inlineChecked = false;
$root = $startTag;
while ($root->parent !== null) {
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation baseline is derived from the outermost ancestor tag by walking $startTag->parent to the top. If the paired tag is nested inside a non-indented wrapper tag (e.g. {if ...} or {define ...} starting at column 1) and then indented inside HTML, $tagIndentLen becomes 0 and the new “preserve tag structural indentation” behavior won’t trigger (it will strip indentation fully again). Consider instead picking the highest ancestor that is still indented (e.g. walk up while the parent exists and parent->position->column > 1, or stop once an ancestor is at column 1), so {foreach} inside an outer {if} still preserves the {foreach} line’s indentation level.

Suggested change
while ($root->parent !== null) {
while ($root->parent !== null && $root->parent->position->column > 1) {

Copilot uses AI. Check for mistakes.
Assert::same("<ul>\n\t<li>1</li>\n\t<li>2</li>\n</ul>", $result);
});


Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new tests cover {foreach} and nested {foreach}/{if} indented directly in HTML, but they don’t cover the common case where these paired tags are wrapped by an outer tag at column 1 (e.g. {if} / {block} / {define} around the <ul>). Adding a regression test for “paired tag inside HTML inside an outer (column-1) tag” would help ensure indentation preservation works when there is a non-indented ancestor tag.

Suggested change
test('paired tag inside HTML inside outer column-1 tag', function () {
$result = dedent("{if true}\n<ul>\n\t{foreach \$items as \$item}\n\t\t<li>{\$item}</li>\n\t{/foreach}\n</ul>\n{/if}", ['items' => ['a', 'b']]);
Assert::same("<ul>\n\t<li>a</li>\n\t<li>b</li>\n</ul>\n", $result);
});

Copilot uses AI. Check for mistakes.
@davidkrmela davidkrmela force-pushed the dedent-preserve-structural-indentation branch from e3196a2 to 8e016c5 Compare April 4, 2026 12:20
@davidkrmela
Copy link
Copy Markdown
Contributor Author

This turned out to be more complex than initially expected. There are two separate issues:

1. Structural indentation not preserved for paired tags inside HTML
When {foreach} or {if} sits indented inside HTML (e.g. inside <ul>), dedent was stripping the entire common indentation prefix instead of only the excess beyond the tag's position. Fixed by using the root ancestor tag's column position as the baseline.

2. Indentation inside HTML elements not reached by dedent
When HTML elements like <ul> are nested inside paired tags (e.g. {if $item}\n\t\t<ul>\n\t\t\t<li>...), the indentation TextNodes before <li> live inside the ElementNode's content fragment — the flat algorithm in applyDedent never reaches them. Added collectHtmlIndentNodes to recursively walk into HTML element content and apply the same dedent baseline.

Both fixes work together so that templates like the one from the blog article produce correct output.

@dg
Copy link
Copy Markdown
Member

dg commented Apr 4, 2026

I just want to ask quickly: Are you using the latest stable version or the current master branch with patch #412 #413

@davidkrmela
Copy link
Copy Markdown
Contributor Author

I tested both:

Latest stable (v3.1.3) produces for:

{var $items = [1, 2, 3]}                                                                                                               
<ul>                                                                                                                                   
    {foreach $items as $item}                                                                                                          
        <li>{$item}</li>
    {/foreach}
</ul>
<ul>
<li>1</li>
<li>2</li>
<li>3</li>
</ul>

Current master (with #412 #413 fixes) produces:

<ul>
        <li>1</li>
        <li>2</li>
        <li>3</li>
</ul>

Neither version produces the correct result as described in that blog post:

<ul>
    <li>...</li>
    <li>...</li>
</ul>

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

Successfully merging this pull request may close these issues.

3 participants