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

Ember / Handlebars: Do not break opening mustache and path #10586

Merged
merged 8 commits into from
Mar 29, 2021

Conversation

dcyriller
Copy link
Collaborator

@dcyriller dcyriller commented Mar 22, 2021

Description

In Ember ecosystem, the opening mustache and the path are often printed on a same line (see this comment and this comment for more details).

Changes

{{!-- Input --}}
<GlimmerComponent
  @errors={{or this.aVeryLongProperty (and this.aProperty (v-get bike "number" "message"))}}
  data-test-beneficiary-account-number
/>
<GlimmerComponent
  @progress={{aPropertyEngdingAfterEightiethColumnToHighlightAWeirdClosingParenIssue}}
/>

{{!-- Prettier stable --}}
<GlimmerComponent
  @errors={{
    or
    this.aVeryLongProperty
    (and this.aProperty (v-get bike "number" "message"))
  }}
  data-test-beneficiary-account-number
/>
<GlimmerComponent
  @progress={{
    aPropertyEngdingAfterEightiethColumnToHighlightAWeirdClosingParenIssue
  }}
/>

{{!-- Prettier main --}}
<GlimmerComponent
  @errors={{or
    this.aLongProperty
    (and this.aProperty (v-get bike "number" "message"))
  }}
  data-test-beneficiary-account-number
/>
<GlimmerComponent
  @progress={{aPropertyEngdingAfterEightiethColumnToHighlightAWeirdClosingParenIssue}}
/>

Checklist

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

@thorn0
Copy link
Member

thorn0 commented Mar 22, 2021

Will this close #8691?

@dcyriller
Copy link
Collaborator Author

It only covers a part of it

@thorn0
Copy link
Member

thorn0 commented Mar 22, 2021

Please add test cases from that issue. If you want, you can prefix the test files' names with issue-8691 (e.g. issue-8691-if-1.hbs). I usually do that when I collect test cases from issues..

@dcyriller
Copy link
Collaborator Author

Please add test cases from that issue. If you want, you can prefix the test files' names with issue-8691 (e.g. issue-8691-if-1.hbs). I usually do that when I collect test cases from issues..

I have a follow-up PR ready for the linked issue. Do you mind if I apply this comment to the follow-up PR?

Indeed, I split the work in two PRs to have two different changelog entries. (So that it is clearer for the end user.) But if you prefer I can merge them. Let me know!

@thorn0
Copy link
Member

thorn0 commented Mar 22, 2021

two different changelog entries. (So that it is clearer for the end user

This is a good idea. I meant only those test cases that this PR improves. Not all the test cases from that issue.

@dcyriller
Copy link
Collaborator Author

I have added test cases taken from #8691 (it won't close the issue though) and rebased to highlight the difference (all commits are supposed to pass the CI).

@thorn0
Copy link
Member

thorn0 commented Mar 24, 2021

Wrapping the closing }} like this looks unexpected.

Prettier pr-10586
Playground link

--parser glimmer
--print-width 90

Input:

<GlimmerComponent
  @errors={{or this.aVeryLongProperty (and this.aProperty (v-get bike "number" "message"))}}
  data-test-beneficiary-account-number
/>

Output:

<GlimmerComponent
  @errors={{or this.aVeryLongProperty (and this.aProperty (v-get bike "number" "message"))
  }}
  data-test-beneficiary-account-number
/>

@dcyriller
Copy link
Collaborator Author

nice catch - I pushed some changes (they specifically adress the issue reported in this comment)

@thorn0 thorn0 merged commit 4407d7b into prettier:main Mar 29, 2021
@dcyriller dcyriller deleted the curly branch March 29, 2021 13:00
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants