Skip to content

Fix nested partial indenting#646

Merged
sunng87 merged 3 commits into
sunng87:masterfrom
cmrschwarz:nested_partial_indenting
Jun 16, 2024
Merged

Fix nested partial indenting#646
sunng87 merged 3 commits into
sunng87:masterfrom
cmrschwarz:nested_partial_indenting

Conversation

@cmrschwarz

@cmrschwarz cmrschwarz commented Jun 15, 2024

Copy link
Copy Markdown
Contributor

Currently, nested partials don't indent correctly.
This is due to two problems:

  • The indent in RenderContextInner is not built up correctly
  • After a stanalone partial, the next raw text is trimmed, so it will not be indented (no leading newline),
    causing it to be misindented.

This PR addresses both issues, causing a simple nested example (that was also added as a test case) to be indented correctly.

Example Input

{{#*inline "nested_partial"}}
<div>
    content
</div>
{{/inline}}

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

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

Output Before

<div>
    <div>
        <div>
        content
    </div>
</div>
</div>

Output After

<div>
    <div>
        <div>
            content
        </div>
    </div>
</div>

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 81.561% (+0.04%) from 81.519%
when pulling 18eeb6c on cmrschwarz:nested_partial_indenting
into 5dda501 on sunng87:master.

@sunng87

sunng87 commented Jun 16, 2024

Copy link
Copy Markdown
Owner

LGTM. Thank you for fixing this long waited issue with my implementation.

The only concern is by adding a new variant to TemplateElement, we introduce a breaking change. The library is written from early days (2015) of rust when there is no #[non_exhaustive]. I should definitely add them to all public enums.

@sunng87 sunng87 merged commit 5513cfa into sunng87:master Jun 16, 2024
@cmrschwarz cmrschwarz mentioned this pull request Jun 16, 2024
@cmrschwarz cmrschwarz deleted the nested_partial_indenting 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