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

Fix indent after if #650

Closed
wants to merge 4 commits into from
Closed

Conversation

cmrschwarz
Copy link
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

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

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
Copy link
Contributor Author

cmrschwarz commented Jun 16, 2024

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
Copy link
Owner

sunng87 commented Jun 22, 2024

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
Copy link
Contributor Author

cmrschwarz commented Jun 24, 2024

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
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.

None yet

3 participants