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: in conditionals, don't render anything after an else branch #671

Merged
merged 6 commits into from Feb 18, 2024

Conversation

joel-hamilton
Copy link
Contributor

@joel-hamilton joel-hamilton commented Feb 16, 2024

This PR enforces a simple rule: don't render anything after an else branch.

There are several bugs currently where if, unless and case conditionals can have branches that occur after an else render, or that branches after an else can actually cause the else branch itself not to render.

Fixes: #670 and #672

Example

<p>If</p>
{% if false %}
  don't show
  {%- else -%}
  show
  {%- else -%}
  don't show
{% endif %}

<p>If with Elsif</p>
{% if false %}
  don't show
  {%- else -%}
  show
  {%- elsif true -%}
  don't show
{% endif %}

<p>Unless</p>
{% unless true %}
  don't show
  {%- else -%}
  show
  {%- else -%}
  don't show
{% endunless %}

<p>Case</p>
{% case true %}
  {%- when false- %}
    don't show
  {%- else -%}
    show
  {%- else -%}
    don't show
{% endcase %}

<p>Case with When</p>
{% case true -%}
  {% when false -%}
    don't show (false)
  {%- else -%}
    show
  {%- else -%}
    don't show (second else)
  {%- when true -%}
    don't show (truthy when condition after else)
  {%- when true -%}
    don't show (truthy when condition after else)
{%- endcase %}

master Behaviour:

<p>If</p>
showdon't show

<p>If with Elsif</p>
don't show

<p>Unless</p>
showdon't show

<p>Case</p>
showdon't show

<p>Case with When</p>
don't show (truthy when condition after else)don't show (truthy when condition after else)

PR Behaviour:

<p>If</p>
show

<p>If with Elsif</p>
show

<p>Unless</p>
show

<p>Case</p>
show
 
<p>Case with When</p>
show

@coveralls
Copy link

coveralls commented Feb 16, 2024

Pull Request Test Coverage Report for Build 7946818540

Details

  • -2 of 27 (92.59%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.09%) to 99.375%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/tags/if.ts 8 10 80.0%
Totals Coverage Status
Change from base Build 7895611436: -0.09%
Covered Lines: 2261
Relevant Lines: 2270

💛 - Coveralls

@joel-hamilton joel-hamilton marked this pull request as ready for review February 16, 2024 03:18
src/tags/if.ts Show resolved Hide resolved
src/tags/case.ts Outdated Show resolved Hide resolved
src/tags/case.ts Outdated Show resolved Hide resolved
test/e2e/issues.spec.ts Outdated Show resolved Hide resolved
@joel-hamilton joel-hamilton changed the title fix: only render the first 'else' template in the case of multiples fix: in conditionals, don't render anything after an else branch Feb 18, 2024
src/tags/case.ts Outdated Show resolved Hide resolved
src/tags/if.ts Outdated Show resolved Hide resolved
src/tags/unless.ts Outdated Show resolved Hide resolved
@harttle harttle merged commit f816955 into harttle:master Feb 18, 2024
11 checks passed
@harttle
Copy link
Owner

harttle commented Feb 18, 2024

@all-contributors please add @joel-hamilton for code

Copy link
Contributor

@harttle

I've put up a pull request to add @joel-hamilton! 🎉

github-actions bot pushed a commit that referenced this pull request Feb 18, 2024
## [10.10.1](v10.10.0...v10.10.1) (2024-02-18)

### Bug Fixes

* in conditionals, don't render anything after an else branch ([#671](#671)) ([f816955](f816955))
* Rely on equal for computing contains ([#668](#668)) ([1937aa1](1937aa1))
Copy link

🎉 This PR is included in version 10.10.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@joel-hamilton joel-hamilton deleted the issue-670 branch February 18, 2024 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sequential else tags all render
3 participants