-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Upgrade wp-prettier to 2.6.2 #40542
Upgrade wp-prettier to 2.6.2 #40542
Conversation
Size Change: +63 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
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.
Couldn't review any changes on this page because GitHub kept freezing the browser when trying to show the diff, but the changes to the parser code look good 👍
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.
Took a little while,eventually got through all the files, the vast majorjty, 99% are line length changes, shorter, or longer
Used the hide whitespace view to review https://github.com/WordPress/gutenberg/pull/40542/files?w=1
I commented on 4 file changes out of the 568 files, so looks pretty good to me
@@ -36,7 +36,8 @@ const TRANSLATION_FUNCTIONS = new Set( [ '__', '_x', '_n', '_nx' ] ); | |||
* | |||
* @type {RegExp} | |||
*/ | |||
const REGEXP_SPRINTF_PLACEHOLDER = /%(((\d+)\$)|(\(([$_a-zA-Z][$_a-zA-Z0-9]*)\)))?[ +0#-]*\d*(\.(\d+|\*))?(ll|[lhqL])?([cduxXefgsp%])/g; | |||
const REGEXP_SPRINTF_PLACEHOLDER = |
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 regex explainer comments alignment below this line can be fixed in a follow up PR
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.
Fixed in an extra commit, thanks for spotting this 👍
return createElement( icon, ( { | ||
return createElement( icon, { | ||
size, | ||
...additionalProps, | ||
} as unknown ) as P ); | ||
} as unknown as P ); |
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.
Parenthesis removed
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.
Yes, that's what the updated Prettier does to all TypeScript X as Y as Z
expressions. There are no longer any parens.
@@ -13,7 +13,7 @@ const prettierPackage = require( require.resolve( 'prettier/package.json' ) ); | |||
|
|||
const isWPPrettier = prettierPackage.name === 'wp-prettier'; | |||
const customOptions = isWPPrettier | |||
? { parenSpacing: true, jsxBracketSameLine: false } | |||
? { parenSpacing: true, bracketSameLine: false } |
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.
bracketSameLine
is the same now in both cases so we can move it to the config
object.
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.
Cleaned up 👍
a58ee6e
to
2eae7d2
Compare
Can we add the merge commit to |
Will do! Thanks for mentioning this, I didn't know that |
This PR requires another Prettier run to format files changed in the |
f4237c7
to
af5ae53
Compare
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.
Let's move forward with this Prettier fork's upgrade. It will require another rebase and Prettier formatting run before landing.
Can we add the merge commit to .git-blame-ignore-revs once merged?
https://github.com/WordPress/gutenberg/blob/trunk/.git-blame-ignore-revs
Let's remember about the follow-up suggested by @noisysocks.
Added with d25c39d. I have just noticed triple |
I updated the
wp-prettier
fork to be in sync with the latest upstream version, Prettier 2.6.2, and published it as[email protected]
.This PR updates the Gutenberg dependency and reformats the codebase.