sunng87 / handlebars-rust

Rust templating with Handlebars
MIT License
1.26k stars 137 forks source link

Fix indent after if #650

Closed cmrschwarz closed 2 months ago

cmrschwarz commented 3 months ago

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 commented 3 months ago

Coverage Status

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

coveralls commented 3 months ago

Coverage Status

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

cmrschwarz commented 3 months ago

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>
sunng87 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

Superseded by #654