Skip to content

fix(glimmer): {{else}}{{#if}} into {{else if}} merging#6080

Merged
alexander-akait merged 4 commits intoprettier:masterfrom
dcyriller:fix-nested-if
May 2, 2019
Merged

fix(glimmer): {{else}}{{#if}} into {{else if}} merging#6080
alexander-akait merged 4 commits intoprettier:masterfrom
dcyriller:fix-nested-if

Conversation

@dcyriller
Copy link
Copy Markdown
Collaborator

@dcyriller dcyriller commented Apr 25, 2019

Description

The first commit highlights the issue. The second fixes it.

The following code should be left untouched:

{{#if a}}
  a
{{else}}
  {{#if c}}
    c
  {{/if}}
  e
{{/if}}

Instead, it was turned into:

{{#if a}}
  a
{{else if c}}
  c
e
{{/if}}

Checks

  • I’ve added tests to confirm my change works.
  • (If the change is user-facing) I’ve added my changes to the CHANGELOG.unreleased.md file following the template.
  • I’ve read the contributing guidelines.

Playground

Try the playground for this PR

Copy link
Copy Markdown

@Turbo87 Turbo87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome!! 👍

Copy link
Copy Markdown
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Need add note in changelog

@dcyriller
Copy link
Copy Markdown
Collaborator Author

@evilebottnawi done, sorry for that

@dcyriller dcyriller force-pushed the fix-nested-if branch 2 times, most recently from ea48afb to 26e6d8e Compare April 29, 2019 21:03
@dcyriller dcyriller changed the title Fix {{else}}{{#if}} into {{else if}} merging fix(glimmer): {{else}}{{#if}} into {{else if}} merging Apr 29, 2019
dcyriller added 2 commits May 1, 2019 09:28
```
{{#if a}}
  a
{{else}}
  {{#if c}}
    c
  {{/if}}
  e
{{/if}}
```
should be left untouched. Instead, it is turned into:
```
{{#if a}}
  a
{{else if c}}
  c
e
{{/if}}
```
@dcyriller
Copy link
Copy Markdown
Collaborator Author

@evilebottnawi CHANGELOG.unreleased.md is up to date.

@alexander-akait
Copy link
Copy Markdown
Member

@dcyriller Thanks, can you fix CI?

@dcyriller
Copy link
Copy Markdown
Collaborator Author

@evilebottnawi CI is now 🍏

I see that you squash the commits, so I don't bother to remove the last one (that was empty).

@alexander-akait
Copy link
Copy Markdown
Member

/cc @prettier/core we need rule about merging (minimum 2 approve from core as example)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants