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
[handlebars] group params in opening block statements #14067
[handlebars] group params in opening block statements #14067
Conversation
7e1d6c3
to
9112a12
Compare
9112a12
to
f048ddc
Compare
a91facb
to
e88b6c0
Compare
@jamescdavis I don't use handlebars, but is this a bug? Prettier 2.8.1 --parser glimmer Input: {{#block a}}
1
{{else if b}}
2
{{else}}
3
{{/block}} Output: {{#block a}}
1
{{else}}{{#if b}}
2
{{else}}
3
{{/if}}{{/block}} I didn't mean |
@jamescdavis I made some changes, please take a look. |
@fisker, your changes look good. Thanks for simplifying the logic! |
As far as the closing block alignment, it's due to the When {{else if b}} is rewritten to {{else}}{{#if b}} and {{/block}} is rewritten to {{/if}}{{/block}} and See this playground link with You also see that explicitly writing this with newlines works fine, even with In any case, this would only be an issue when the If you wanted something like: {{#block a}}
1
{{else}}
{{#if b}}
2
{{/if}}
{{/block}} You'd explicitly write it that way. And {{#block a}}
1
{{else}}
{{#if b}}
2
{{/if}}
{{else}}
3
{{/block}} doesn't even make sense since there are two Maybe it should not try to match the opening block keyword and never split {{#block a}}
1
{{else if b}}
2
{{else}}
3
{{/block}} as-is. |
@fisker I did notice one issue after your changes: when the
Input: {{#if a}}
1
{{else if b}}
2
{{/if}} Output: {{#if
a
}}
1
{{else
if
b
}}
2
{{/if}} Expected: {{#if
a
}}
1
{{else
if
b
}}
2
{{/if}} I've added a failing test and a fix for this. But also, maybe it shouldn't wrap here? Maybe It makes more of a difference with longer block keywords. Which do you think is better? (assuming Input: {{#abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrst param param param as |foo|}}
1
{{else abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrst param param param as |bar|}}
2
{{/abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrst}} Ouput with else "if" grouped: {{#abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrst
param param param
as |foo|
}}
1
{{else
abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrst
param param param
as |bar|
}}
2
{{/abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrst}} or Output with else "if" not grouped: {{#abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrst
param param param
as |foo|
}}
1
{{else abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrst
param param param
as |bar|
}}
2
{{/abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrstuvwxyz-abcdefghijklmnopqrst}} |
I actually decided to make the change to not group |
@@ -570,6 +624,86 @@ printWidth: 80 | |||
Another thing | |||
{{~/if~}} | |||
|
|||
<div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can put this test in a separate dir with small printWidth, instead of deep.nest.
Agree, but this problem can be addressed in another PR. |
I removed |
I nerver use handlebars, I can't tell. @dcyriller Do you have time to review? We plan to make a release, and I was hoping this to be included. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I have been afk for a few weeks.
@fisker I moved the test as you suggested. With the approval from @dcyriller, this should be GTG? |
@jamescdavis Thanks for your work. |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [prettier](https://prettier.io) ([source](https://github.com/prettier/prettier)) | devDependencies | patch | [`2.8.3` -> `2.8.4`](https://renovatebot.com/diffs/npm/prettier/2.8.3/2.8.4) | --- ### Release Notes <details> <summary>prettier/prettier</summary> ### [`v2.8.4`](https://github.com/prettier/prettier/blob/HEAD/CHANGELOG.md#​284) [Compare Source](prettier/prettier@2.8.3...2.8.4) [diff](prettier/prettier@2.8.3...2.8.4) ##### Fix leading comments in mapped types with `readonly` ([#​13427](prettier/prettier#13427) by [@​thorn0](https://github.com/thorn0), [@​sosukesuzuki](https://github.com/sosukesuzuki)) <!-- prettier-ignore --> ```tsx // Input type Type = { // comment readonly [key in Foo]; }; // Prettier 2.8.3 type Type = { readonly // comment [key in Foo]; }; // Prettier 2.8.4 type Type = { // comment readonly [key in Foo]; }; ``` ##### Group params in opening block statements ([#​14067](prettier/prettier#14067) by [@​jamescdavis](https://github.com/jamescdavis)) This is a follow-up to [#​13930](prettier/prettier#13930) to establish wrapping consistency between opening block statements and else blocks by grouping params in opening blocks. This causes params to break to a new line together and not be split across lines unless the length of params exceeds the print width. This also updates the else block wrapping to behave exactly the same as opening blocks. <!-- prettier-ignore --> ```hbs {{! Input }} {{#block param param param param param param param param param param as |blockParam|}} Hello {{else block param param param param param param param param param param as |blockParam|}} There {{/block}} {{! Prettier 2.8.3 }} {{#block param param param param param param param param param param as |blockParam| }} Hello {{else block param param param param param param param param param param}} There {{/block}} {{! Prettier 2.8.4 }} {{#block param param param param param param param param param param as |blockParam| }} Hello {{else block param param param param param param param param param param as |blockParam| }} There {{/block}} ``` ##### Ignore files in `.sl/` ([#​14206](prettier/prettier#14206) by [@​bolinfest](https://github.com/bolinfest)) In [Sapling SCM](https://sapling-scm.com/), `.sl/` is the folder where it stores its state, analogous to `.git/` in Git. It should be ignored in Prettier like the other SCM folders. ##### Recognize `@satisfies` in Closure-style type casts ([#​14262](prettier/prettier#14262) by [@​fisker](https://github.com/fisker)) <!-- prettier-ignore --> ```jsx // Input const a = /** @​satisfies {Record<string, string>} */ ({hello: 1337}); const b = /** @​type {Record<string, string>} */ ({hello: 1337}); // Prettier 2.8.3 const a = /** @​satisfies {Record<string, string>} */ { hello: 1337 }; const b = /** @​type {Record<string, string>} */ ({ hello: 1337 }); // Prettier 2.8.4 const a = /** @​satisfies {Record<string, string>} */ ({hello: 1337}); const b = /** @​type {Record<string, string>} */ ({hello: 1337}); ``` ##### Fix parens in inferred function return types with `extends` ([#​14279](prettier/prettier#14279) by [@​fisker](https://github.com/fisker)) <!-- prettier-ignore --> ```ts // Input type Foo<T> = T extends ((a) => a is infer R extends string) ? R : never; // Prettier 2.8.3 (First format) type Foo<T> = T extends (a) => a is infer R extends string ? R : never; // Prettier 2.8.3 (Second format) SyntaxError: '?' expected. // Prettier 2.8.4 type Foo<T> = T extends ((a) => a is infer R extends string) ? R : never; ``` </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMjUuMSIsInVwZGF0ZWRJblZlciI6IjM0LjEyOC4zIn0=--> Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1775 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-authored-by: fisker Cheung <lionkay@gmail.com>
Description
This is a follow-up to #13930 to establish wrapping consistency between opening block statements and else blocks by grouping params in opening blocks. This causes params to break to a new line together and not be split across lines unless the length of params exceeds the print width. This also updates the else block wrapping to behave exactly the same as opening blocks (after #13930 it attempts to keep params on same line as the "else block" keywords, even when block params cause wrapping, see
else block
in thePrettier main
example below).Checklist
(If changing the API or CLI) I’ve documented the changes I’ve made (in the.docs/
directory)changelog_unreleased/*/XXXX.md
file followingchangelog_unreleased/TEMPLATE.md
.✨Try the playground for this PR✨