Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,23 @@ _x( '"%1$s"/ %2$s', 'caption' );
_x( '"%1$s"/ %2$s', 'caption' );
`,
},
{
code: ` // translators: %s: Hello at 6:00 AM
i18n.sprintf( i18n.__( 'Hello at %s' ), '6:00 AM' );`,
},
// test for precisions now
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this test because it was mention that, this was causing a false error flag on extra placeholder

Link: #70458 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant the inline comment 😅

// test for precisions now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, my bad 😅

{
code: `// translators: %.2f: Percentage
i18n.sprintf( i18n.__( 'Percentage: %.2f' ), 1.00 );`,
},
{
code: `// translators: %.*f: Percentage
i18n.sprintf( i18n.__( 'Percentage: %.*f' ), 2, 1.00 );`,
},
{
code: `// translators: %(named).2s: truncated name
i18n.sprintf( i18n.__( 'Truncated name: %(named).2s' ), { named: 'Long Name' } );`,
},
],
invalid: [
{
Expand Down
6 changes: 3 additions & 3 deletions packages/eslint-plugin/rules/i18n-translator-comments.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ function extractTranslatorKeys( commentText ) {
while (
( match = REGEXP_COMMENT_PLACEHOLDER.exec( commentBody ) ) !== null
) {
keys.set( match[ 1 ], keys.get( match[ 1 ] ) || match[ 2 ] === ':' );
const rawKey = match[ 1 ];
const hasColon = match.groups?.colon?.trim() === ':';
keys.set( rawKey, keys.get( rawKey ) || hasColon );
}

return keys;
Expand Down Expand Up @@ -211,8 +213,6 @@ module.exports = {
} )
: [];

// console.log({extra, keysInComment, placeholdersUsed});

if ( extra.length > 0 ) {
context.report( {
node,
Expand Down
50 changes: 39 additions & 11 deletions packages/eslint-plugin/utils/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,47 @@ const REGEXP_SPRINTF_PLACEHOLDER_UNORDERED =
/(?:(?<!%)%[+-]?(?:(?:0|'.)?-?[0-9]*(?:\.(?:[ 0]|'.)?[0-9]+)?|(?:[ ])?-?[0-9]+(?:\.(?:[ 0]|'.)?[0-9]+)?)[bcdeEfFgGosuxX])/;

/**
* Regular expression matching comment placeholders.
*
* /(?:^|\s|,)\s*(%[sdf]|%?[a-zA-Z0-9_]+|%[0-9]+\$?[sdf]{0,1})(:)?/g;
* ▲ ▲ ▲ ▲ ▲
* │ │ │ │ │
* │ │ │ │ └─ Match colon at the end of the placeholder ( optional )
* │ │ │ └─ Match a Index placeholder but allow variations (e.g. %1, %1s, %1$d)
* │ │ └─ Match a placeholder with index or named argument (e.g. %1, %name, %2)
* │ └─ Match Unamed placeholder (e.g. %s, %d)
* └─ Match the start of string, whitespace, or comma
* Regular expression to extract placeholder keys from translator comments.
*
* It matches common i18n placeholders and comment-only keys, with optional
* precision and a trailing colon (indicating a description).
*
* Breakdown of the regex:
*```md
* (?:^|\s|,) — Non-capturing group that matches the start of the string, a whitespace character, or a comma (ensures proper separation).
*
* \s* — Optional whitespace after the separator.
*
* ( — Capturing group for the full placeholder (used as key):
* %? — Optional `%` to allow bare keys like `1`, `label` in comments.
* ( — Group for matching placeholder variants:
* \(?<named>[a-zA-Z_][a-zA-Z0-9_]*\) — Named placeholder in the form: %(name)
* (?:\.\d+|\.\*)? — Optional precision: .2 or .*
* [sdf] — Format specifier: s, d, or f
*
* |
* (?<positional>[1-9][0-9]*)\$? — Positional placeholder like %1$
* (?:\.\d+|\.\*)? — Optional precision
* [sdf] — Format specifier
*
* | — OR
* (?:\.\d+|\.\*)?[sdf] — Unnamed placeholder with optional precision
*
* | [1-9][0-9]* — Bare positional key like `1`, `2`
* | [sdf] — Just a format type
* | [a-zA-Z_][a-zA-Z0-9_]* — Bare named key (used in comments)
* )
* )
*
* (?<colon>:[ \t]+)? — Optional named group `colon`, matches a colon followed by space or tab,
* indicating that this placeholder has a description in the comment.
*
* Flags:
* g — global, so it matches all placeholders in the comment string.
* ```
*/
const REGEXP_COMMENT_PLACEHOLDER =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel a top-level arrow-based breakdown (similar to the one in REGEXP_SPRINTF_PLACEHOLDER example) would improve readability for the regex. I think the detailed explanation (if needed) could still remain at the bottom.

Having those quick visual markers at the top makes it much easier to parse at a glance. WDYT?

const REGEXP_SPRINTF_PLACEHOLDER =
/(?<!%)%(((\d+)\$)|(\(([$_a-zA-Z][$_a-zA-Z0-9]*)\)))?[ +0#-]*\d*(\.(\d+|\*))?(ll|[lhqL])?([cduxXefgsp])/g;
// ▲ ▲ ▲ ▲ ▲ ▲ ▲ type
// │ │ │ │ │ └ Length (unsupported)
// │ │ │ │ └ Precision / max width
// │ │ │ └ Min width (unsupported)
// │ │ └ Flags (unsupported)
// └ Index └ Name (for named arguments)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did also thought of that but when I tried to write that due to the regex length, the visual marker syntax felt awkward to read as it was getting hard to mark the points that I wanted to explain, Hence I took this approach as it allowed me to break the regex into a way, I thought I could explain it

/(?:^|\s|,)\s*(%[sdf]|%?[a-zA-Z0-9_]+|%[0-9]+\$?[sdf]{0,1})(:)?/g;
/(?:^|\s|,)\s*(%?(?:\((?<named>[a-zA-Z_][a-zA-Z0-9_]*)\)(?:\.\d+|\.\*)?[sdf]|(?<positional>[1-9][0-9]*)\$?(?:\.\d+|\.\*)?[sdf]|(?:\.\d+|\.\*)?[sdf]|[1-9][0-9]*|[sdf]|[a-zA-Z_][a-zA-Z0-9_]*))(?<colon>:[ \t]+)?/g;

module.exports = {
TRANSLATION_FUNCTIONS,
Expand Down
Loading