-
Notifications
You must be signed in to change notification settings - Fork 22
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
prettierx: Merge Prettier 2.3.1 updates - draft 2 #569
prettierx: Merge Prettier 2.3.1 updates - draft 2 #569
Conversation
* Start 2.3 blog post * Apply suggestions from code review Co-authored-by: Alex Rattray <[email protected]> * Regenerate the post * Regenerate blog post * Regenerate post * Edit changelog entries * Regenerate blog post Co-authored-by: Alex Rattray <[email protected]> Co-authored-by: fisker Cheung <[email protected]>
* Use async API in CLI * Remove `synchronous-promise`
Fix #5599, long yaml keys trigger explicit mapping
Bumps [@babel/preset-env](https://github.com/babel/babel/tree/HEAD/packages/babel-preset-env) from 7.14.1 to 7.14.2. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.14.2/packages/babel-preset-env) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…893) Bumps [eslint-plugin-import](https://github.com/benmosher/eslint-plugin-import) from 2.22.1 to 2.23.2. - [Release notes](https://github.com/benmosher/eslint-plugin-import/releases) - [Changelog](https://github.com/benmosher/eslint-plugin-import/blob/master/CHANGELOG.md) - [Commits](import-js/eslint-plugin-import@v2.22.1...v2.23.2) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [webpack](https://github.com/webpack/webpack) from 5.36.2 to 5.37.0. - [Release notes](https://github.com/webpack/webpack/releases) - [Commits](webpack/webpack@v5.36.2...v5.37.0) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [esm-utils](https://github.com/fisker/esm-utils) from 1.0.1 to 1.1.0. - [Release notes](https://github.com/fisker/esm-utils/releases) - [Commits](fisker/esm-utils@v1.0.1...v1.1.0) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [@glimmer/syntax](https://github.com/glimmerjs/glimmer-vm) from 0.79.0 to 0.79.1. - [Release notes](https://github.com/glimmerjs/glimmer-vm/releases) - [Changelog](https://github.com/glimmerjs/glimmer-vm/blob/master/CHANGELOG.md) - [Commits](glimmerjs/glimmer-vm@v0.79.0...v0.79.1) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [flow-parser](https://github.com/facebook/flow) from 0.150.1 to 0.151.0. - [Release notes](https://github.com/facebook/flow/releases) - [Changelog](https://github.com/facebook/flow/blob/master/Changelog.md) - [Commits](facebook/flow@v0.150.1...v0.151.0) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Georgii Dolzhykov <[email protected]>
export const bem = block => | ||
export const bem = | ||
block => | ||
/** | ||
* @param {String} [element] - the BEM Element within that block; if undefined, selects the block itself. | ||
* @returns {Function} | ||
*/ | ||
element => |
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.
The indentation in this arrow function block does not look right to me.
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.
From a cursory glance, it looks like this indentation is consistent with Prettier 2.3.1's behaviour: https://github.com/prettier/prettier/blob/2.3.1/tests/format/js/arrows/__snapshots__/jsfmt.spec.js.snap#L2110
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 are right, aargh!
I think we should raise an issue on Prettier, if it was not reported before, and see if we can contribute a bug fix. I would be happy to do this if you like.
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.
Looks like the change in behaviour came in prettier/prettier#9992: prettier/prettier@046ffb1#diff-9fc5f6234a69ade9849cab79fc6c92588186225a105330442778daf68403315bL1836-R2051
There's a related issue, prettier/prettier#11001, regarding print-width, but I don't see much discussion regarding indentation.
tests/format/js/arrows/with-inner-spacing/__snapshots__/jsfmt.spec.js.snap
Outdated
Show resolved
Hide resolved
Closing in favour of #603 |
(in prettierx-update-branch-001) based on some updates from: - #549 - #569 (with very limited update needed in src/language-js/needs-parens.js) TODO items: - include test updates from prettierX dev branch - update documentation Co-authored-by: Adaline Valentina Simonian <[email protected]> Co-authored-by: Christopher J. Brody <[email protected]>
(in prettierx-update-branch-001) based on some updates from: - #549 - #569 (with very limited update needed in src/language-js/needs-parens.js) TODO items: - include test updates from prettierX dev branch - update documentation Co-authored-by: Adaline Valentina Simonian <[email protected]> Co-authored-by: Christopher J. Brody <[email protected]>
to be more consistent with prettierX 0.18.x including some updates from: - #549 - #569 Co-authored-by: Christopher J. Brody <[email protected]> Co-authored-by: Adaline Valentina Simonian <[email protected]>
to be more consistent with prettierX 0.18.x including some updates from: - #549 - #569 Co-authored-by: Christopher J. Brody <[email protected]> Co-authored-by: Adaline Valentina Simonian <[email protected]>
to be more consistent with prettierX 0.18.x including some updates from: - #549 - #569 Co-authored-by: Christopher J. Brody <[email protected]> Co-authored-by: Adaline Valentina Simonian <[email protected]>
(in prettierx-update-branch-0010) based on some updates from: - #549 - #569 (with very limited update needed in src/language-js/needs-parens.js) TODO items: - include test updates from prettierX dev branch - update documentation Co-authored-by: Adaline Valentina Simonian <[email protected]> Co-authored-by: Christopher J. Brody <[email protected]>
to be more consistent with prettierX 0.18.x including some updates from: - #549 - #569 Co-authored-by: Christopher J. Brody <[email protected]> Co-authored-by: Adaline Valentina Simonian <[email protected]>
(in prettierx-update-branch-0010 version branch) as proposed in: - #569 - #603 Co-authored-by: Adaline Valentina Simonian <[email protected]> Co-authored-by: Christopher J. Brody <[email protected]>
to be more consistent with prettierX 0.18.x including some updates from: - #549 - #569 Co-authored-by: Christopher J. Brody <[email protected]> Co-authored-by: Adaline Valentina Simonian <[email protected]>
(in prettierx-update-branch-0010 version branch) as proposed in: - #569 - #603 Co-authored-by: Adaline Valentina Simonian <[email protected]> Co-authored-by: Christopher J. Brody <[email protected]>
to be more consistent with prettierX 0.18.x including some updates from: - #549 - #569 Co-authored-by: Christopher J. Brody <[email protected]> Co-authored-by: Adaline Valentina Simonian <[email protected]>
(in prettierx-rebase-branch-001 version branch) as proposed in: - #569 - #603 Co-authored-by: Adaline Valentina Simonian <[email protected]> Co-authored-by: Christopher J. Brody <[email protected]>
into prettierx-rebase-branch-001 based on some updates from: - #549 - #569 - #603 (with very limited update needed in src/language-js/needs-parens.js) **tested** with prettierX-specific test cases from prettierX 0.18.x TODO items: - include test updates from prettierX dev branch - update documentation Co-authored-by: Adaline Valentina Simonian <[email protected]> Co-authored-by: Christopher J. Brody <[email protected]>
into prettierx-rebase-branch-001 based on some updates from: - #549 - #569 - #603 (with a very limited update needed in src/language-js/needs-parens.js) **tested** with prettierX-specific test cases from prettierX 0.18.x TODO items: - include test updates from prettierX dev branch - update documentation Co-authored-by: Adaline Valentina Simonian <[email protected]> Co-authored-by: Christopher J. Brody <[email protected]>
to be more consistent with prettierX 0.18.x including some updates from: - #549 - #569 - #603 Co-authored-by: Christopher J. Brody <[email protected]> Co-authored-by: Adaline Valentina Simonian <[email protected]>
to be more consistent with prettierX 0.18.x including some updates from: - #549 - #569 - #603 Co-authored-by: Christopher J. Brody <[email protected]> Co-authored-by: Adaline Valentina Simonian <[email protected]>
into prettierx-rebase-branch-001 based on some updates from: - #549 - #569 - #603 (with a very limited update needed in src/language-js/needs-parens.js) **tested** with prettierX-specific formatting test cases from prettierX 0.18.x (discovered a very limited number of changes from prettierX 0.18.x) TODO items: - include test updates from prettierX dev branch - update documentation Co-authored-by: Adaline Valentina Simonian <[email protected]> Co-authored-by: Christopher J. Brody <[email protected]>
into prettierx-rebase-branch-001 based on some updates from: - #549 - #569 - #603 (with a very limited update needed in src/language-js/needs-parens.js) **tested** with prettierX-specific formatting test cases from prettierX 0.18.x (discovered a very limited number of changes from prettierX 0.18.x) TODO items: - include test updates from prettierX dev branch - update documentation Co-authored-by: Adaline Valentina Simonian <[email protected]> Co-authored-by: Christopher J. Brody <[email protected]>
to be more consistent with prettierX 0.18.x including some updates from: - #549 - #569 - #603 Co-authored-by: Christopher J. Brody <[email protected]> Co-authored-by: Adaline Valentina Simonian <[email protected]>
into prettierx-rebase-branch-001 based on some updates from: - #549 - #569 - #603 with a very limited update needed in src/language-js/needs-parens.js and with comments where the updates are NO LONGER NEEDED in src/language-js/needs-parens.js tested with prettierX-specific formatting test cases from prettierX 0.18.x (discovered a very limited number of deviations) TODO items: - include test updates from prettierX dev branch - update documentation Co-authored-by: Adaline Valentina Simonian <[email protected]> Co-authored-by: Christopher J. Brody <[email protected]>
into prettierx-rebase-branch-001 based on some updates from: - #549 - #569 - #603 with a very limited update needed in src/language-js/needs-parens.js and with comments where the updates are NO LONGER NEEDED in src/language-js/needs-parens.js tested with prettierX-specific formatting test cases from prettierX 0.18.x (discovered a very limited number of deviations) TODO items: - include test updates from prettierX dev branch - update documentation Co-authored-by: Adaline Valentina Simonian <[email protected]> Co-authored-by: Christopher J. Brody <[email protected]>
into prettierx 0.19.0-01-update-branch based on some updates from: - #549 - #569 - #603 with a very limited update needed in src/language-js/needs-parens.js and with comments where updates from 0.18.x are NO LONGER NEEDED in src/language-js/needs-parens.js tested with prettierX-specific formatting test cases from prettierX 0.18.x (discovered a very limited number of deviations) TODO items: - include test updates from prettierX dev branch - update documentation Co-authored-by: Adaline Valentina Simonian <[email protected]> Co-authored-by: Christopher J. Brody <[email protected]>
into prettierx 0.19.0-01-update-branch based on some updates from: - #549 - #569 - #603 with a very limited update needed in src/language-js/needs-parens.js and with comments where updates from 0.18.x are NO LONGER NEEDED in src/language-js/needs-parens.js tested with prettierX-specific formatting test cases from prettierX 0.18.x (discovered a very limited number of deviations) TODO items: - include test updates from prettierX dev branch - update documentation Co-authored-by: Adaline Valentina Simonian <[email protected]> Co-authored-by: Christopher J. Brody <[email protected]>
Closes #549
Closes #548
Closes #422
Closes #414
I didn't realise that when I renamed the branch it'd close the other PR. So here's a new one!
From prior PR #549
Many, many hours of work later, this is starting to look good! Still needs work, though.
Merge conflicts have been resolved, moving the appropriate changes in prettierx into their corresponding new locations with the multiple refactors made upstream.
However, about 374 out of 1189 test suites are failing - one, for example, because parentheses are not correctly being printed around multi-line conditionals.These failing tests shouldn't be that hard to resolve; it seems that in most of the cases, at least from a cursory glance, multiple tests are failing due to the same sets of bugs created from the merge. It will take a bit of effort and time to resolve but then again, so does merging a massive amount of changes into a fork and, well, at least that's mostly done now!Tests now pass (at least locally)!Actually, it turns out there are 4 test suites that still fail - I hadn't run them because I was unaware of theTest failures resolved.FULL_TEST
flag.I've enabled edits by maintainers so if y'all would like to have a crack at anything, please, be my guest. In the meantime, I'll keep trying to iron this stuff out.