Skip to content

Fix indent after if#650

Closed
cmrschwarz wants to merge 4 commits into
sunng87:masterfrom
cmrschwarz:fix_indent_after_if
Closed

Fix indent after if#650
cmrschwarz wants to merge 4 commits into
sunng87:masterfrom
cmrschwarz:fix_indent_after_if

Conversation

@cmrschwarz

Copy link
Copy Markdown
Contributor

Expands on #646 to fix indentation for {{#if}} blocks.

Example Input

{{#*inline "partial"}}
<div>
    {{#if foo}}
    foobar
    {{/if}}
</div>
{{/inline}}

<div>
    {{>partial}}
</div>

Output Before

<div>
    <div>
    foobar
</div>
</div>

Output Now

<div>
    <div>
        foobar
    </div>
</div>

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 81.653% (+0.09%) from 81.561%
when pulling bebee0f on cmrschwarz:fix_indent_after_if
into 89e1c0c on sunng87:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 81.673% (+0.1%) from 81.561%
when pulling 8f34fa7 on cmrschwarz:fix_indent_after_if
into 89e1c0c on sunng87:master.

@cmrschwarz

cmrschwarz commented Jun 16, 2024

Copy link
Copy Markdown
Contributor Author

Hm, this is a little overzealous now if blocks don't contain direct text at all.
Do you have an idea for a better criterion on when to insert the leading indent?
We could also just merge the first part of this for now and think about tackling the more genral case later.
((not indended to be merged in this state))

Breaking Counterexample

{{#*inline "nested_partial"}}
<div>
    foobar
</div>
{{/inline}}
{{#*inline "partial"}}
<div>
    {{#if foo}}
    {{#if foo}}
    {{> nested_partial}}
    {{/if}}
    {{/if}}
</div>
{{/inline}}
<div>
    {{>partial}}
</div>

Expected Output

<div>
    <div>
        <div>
            foobar
        </div>
    </div>
</div>

Actual Output

<div>
    <div>
            <div>
            foobar
        </div>
    </div>
</div>

@cmrschwarz cmrschwarz marked this pull request as draft June 16, 2024 15:14
@sunng87

sunng87 commented Jun 22, 2024

Copy link
Copy Markdown
Owner

Just had a quick look at the template of nested if case

[src/registry.rs:789:9] &tpl = Template {
    name: None,
    elements: [
        RawString(
            "\n",
        ),
        DecoratorBlock(
            DecoratorTemplate {
                name: Name(
                    "inline",
                ),
                params: [
                    Literal(
                        String("nested_partial"),
                    ),
                ],
                hash: {},
                template: Some(
                    Template {
                        name: None,
                        elements: [
                            RawString(
                                "<div>\n    foobar\n</div>\n",
                            ),
                        ],
                        mapping: [
                            TemplateMapping(
                                3,
                                1,
                            ),
                        ],
                    },
                ),
                indent: None,
            },
        ),
        RawString(
            "",
        ),
        DecoratorBlock(
            DecoratorTemplate {
                name: Name(
                    "inline",
                ),
                params: [
                    Literal(
                        String("partial"),
                    ),
                ],
                hash: {},
                template: Some(
                    Template {
                        name: None,
                        elements: [
                            RawString(
                                "<div>\n",
                            ),
                            HelperBlock(
                                HelperTemplate {
                                    name: Name(
                                        "if",
                                    ),
                                    params: [
                                        Path(
                                            Relative(
                                                (
                                                    [
                                                        Named(
                                                            "foo",
                                                        ),
                                                    ],
                                                    "foo",
                                                ),
                                            ),
                                        ),
                                    ],
                                    hash: {},
                                    block_param: None,
                                    template: Some(
                                        Template {
                                            name: None,
                                            elements: [
                                                Indent,
                                                RawString(
                                                    "",
                                                ),
                                                HelperBlock(
                                                    HelperTemplate {
                                                        name: Name(
                                                            "if",
                                                        ),
                                                        params: [
                                                            Path(
                                                                Relative(
                                                                    (
                                                                        [
                                                                            Named(
                                                                                "foo",
                                                                            ),
                                                                        ],
                                                                        "foo",
                                                                    ),
                                                                ),
                                                            ),
                                                        ],
                                                        hash: {},
                                                        block_param: None,
                                                        template: Some(
                                                            Template {
                                                                name: None,
                                                                elements: [
                                                                    Indent,
                                                                    RawString(
                                                                        "    ",
                                                                    ),
                                                                    PartialExpression(
                                                                        DecoratorTemplate {
                                                                            name: Name(
                                                                                "nested_partial",
                                                                            ),
                                                                            params: [],
                                                                            hash: {},
                                                                            template: None,
                                                                            indent: Some(
                                                                                "    ",
                                                                            ),
                                                                        },
                                                                    ),
                                                                    RawString(
                                                                        "",
                                                                    ),
                                                                ],
                                                                mapping: [
                                                                    TemplateMapping(
                                                                        11,
                                                                        5,
                                                                    ),
                                                                    TemplateMapping(
                                                                        11,
                                                                        5,
                                                                    ),
                                                                    TemplateMapping(
                                                                        11,
                                                                        5,
                                                                    ),
                                                                    TemplateMapping(
                                                                        12,
                                                                        5,
                                                                    ),
                                                                ],
                                                            },
                                                        ),
                                                        inverse: None,
                                                        block: true,
                                                        chain: false,
                                                    },
                                                ),
                                                RawString(
                                                    "",
                                                ),
                                            ],
                                            mapping: [
                                                TemplateMapping(
                                                    10,
                                                    5,
                                                ),
                                                TemplateMapping(
                                                    10,
                                                    5,
                                                ),
                                                TemplateMapping(
                                                    10,
                                                    5,
                                                ),
                                                TemplateMapping(
                                                    13,
                                                    5,
                                                ),
                                            ],
                                        },
                                    ),
                                    inverse: None,
                                    block: true,
                                    chain: false,
                                },
                            ),
                            Indent,
                            RawString(
                                "</div>\n",
                            ),
                        ],
                        mapping: [
                            TemplateMapping(
                                8,
                                1,
                            ),
                            TemplateMapping(
                                9,
                                5,
                            ),
                            TemplateMapping(
                                14,
                                1,
                            ),
                            TemplateMapping(
                                14,
                                1,
                            ),
                        ],
                    },
                ),
                indent: None,
            },
        ),
        RawString(
            "<div>\n    ",
        ),
        PartialExpression(
            DecoratorTemplate {
                name: Name(
                    "partial",
                ),
                params: [],
                hash: {},
                template: None,
                indent: Some(
                    "    ",
                ),
            },
        ),
        Indent,
        RawString(
            "</div>\n",
        ),
    ],
    mapping: [
        TemplateMapping(
            1,
            1,
        ),
        TemplateMapping(
            2,
            1,
        ),
        TemplateMapping(
            7,
            1,
        ),
        TemplateMapping(
            7,
            1,
        ),
        TemplateMapping(
            16,
            1,
        ),
        TemplateMapping(
            17,
            5,
        ),
        TemplateMapping(
            18,
            1,
        ),
        TemplateMapping(
            18,
            1,
        ),
    ],
}

Perhaps we want to avoid the Indent of first if ?

@cmrschwarz

cmrschwarz commented Jun 24, 2024

Copy link
Copy Markdown
Contributor Author

Yep, that's the issue.

The first {{if}} shouldn't indent, but the second one should, despite both containing a single nested block.
So we need to come up with some rule that we can put in code that leads to the correct behavior.

The correct solution is probably to just make templates/partials themselves start with an Indent element
(unless their head isn't standalone),
but I think that would then still indent one level too far. Maybe we could give the Indent element an option to
use the parent indent instead of the one of the current block? Though that does not seem like it would win any beauty prizes either.

Do you have a better idea? Otherwise I think I could get to trying this approach some time this week.

@cmrschwarz cmrschwarz mentioned this pull request Jun 30, 2024
@cmrschwarz

Copy link
Copy Markdown
Contributor Author

Superseded by #654

@cmrschwarz cmrschwarz closed this Jun 30, 2024
@cmrschwarz cmrschwarz deleted the fix_indent_after_if branch July 13, 2024 20:33
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