Extend valid-sprintf ESLint rule to catch placeholders that should be numbered #20574
Conversation
swissspidy
requested review from
ajitbohra,
ellatrix,
gziolo,
nerrad,
ntwb and
youknowriad
as code owners
swissspidy
added
the
Internationalization (i18n)
|
Size Change: +727 B (0%) Total Size: 885 kB
ℹ️ View Unchanged
|
|
Nice! I can plan to take a look at this and #20554 You may also be interested in Specifically, there is this rule: I would be curious to have your opinion: Both this pull request and the above |
|
@aduth Awesome, thank you!
The issues with named parameters are probably a) that they are misleading (people could mistakenly translate them), and b) that they are not natively supported in PHP. Encouraging named placeholders would diverge the (WordPress) JS and PHP code base in a way that there would be duplicate strings and translators would have to learn two different styles of placeholders. |
Huh! For some reason, I thought that they were. I can see how this is an issue. |
swissspidy
force-pushed
the
fix/eslint-plugin-valid-sprintf-rule
branch
from
ef6b0a3
to
3ae191b
Compare
…intf-rule # Conflicts: # packages/eslint-plugin/rules/i18n-translator-comments.js # packages/eslint-plugin/rules/valid-sprintf.js # packages/eslint-plugin/utils/index.js
| const nextStep = sprintf( | ||
| '\nRun %s to build the latest version of %s, then open %s to get started!\n', | ||
| '\nRun %1$s to build the latest version of %2$s, then open %3$s to get started!\n', |
There was a problem hiding this comment.
I don't know that we need to do it within the scope of this pull request, but we should enhance the rule to consider sprintf only from @wordpress/i18n, as a "solution" to allow the original usage here.
|
One thing I missed in review here is that because we added a new See: #21610 |
Description
This bug fix / enhancement extends the
@wordpress/valid-sprintfESLint rule to detectsprintfusage with multiple unordered placeholders, which is a bad practice.With this PR, this leads to a better experience for translators and users, as otherwise it might be impossible to translate certain strings correctly.
It does not contain a fixer for it, as it couldn't be reliable—especially when using a mix of unordered and ordered placeholders.
NB: I also updated the code to use ESLint's
messageIdfeature to make the rules & tests easier to read.Fixes #20554.
Note: This PR builds on top of the changes in #20555, which should preferably be merged first.
How has this been tested?
Extended unit tests and ran the new rules against Gutenberg code base.
Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: