Skip to content

Comments

fix(l10n-plugin): also check translation strings in n method#1112

Merged
susnux merged 1 commit intomainfrom
fix/l10n-also-for-n
Jul 31, 2025
Merged

fix(l10n-plugin): also check translation strings in n method#1112
susnux merged 1 commit intomainfrom
fix/l10n-also-for-n

Conversation

@susnux
Copy link
Contributor

@susnux susnux commented Jul 31, 2025

  • also check n method
  • use CallExpression for visitor (there are more Literals than call expressions, so this should boost performance).

- also check `n` method
- use `CallExpression` for visitor (there are more Literals than call
  expressions, so this should boost performance).

Signed-off-by: Ferdinand Thiessen <[email protected]>
@susnux susnux requested review from Antreesy and ShGKme July 31, 2025 14:29
@susnux susnux added bug Something isn't working 3. to review labels Jul 31, 2025
@susnux susnux merged commit 7f6f46f into main Jul 31, 2025
7 checks passed
@susnux susnux deleted the fix/l10n-also-for-n branch July 31, 2025 14:31
@ShGKme
Copy link
Contributor

ShGKme commented Jul 31, 2025

Unrelated to this PR specifically, but I was sure that we must have a space before symbol.

@Antreesy
Copy link
Contributor

Also unrelated, but there was a 4th undefined argument to these functions, usually app, string, params, undefined, { escape: false}
Should we add a rule to warn on it?

@susnux
Copy link
Contributor Author

susnux commented Jul 31, 2025

Also unrelated, but there was a 4th undefined argument to these functions, usually app, string, params, undefined, { escape: false}

Do you mean t(app, text, params, number, options) which is deprecated in favor of t(app, params, options) ?
I do not think we need to warn on every deprecated of a library we maintain, before you upgrade dependencies you should check the release notes and adjust your code. In this case l10n get rather few updates.

Unrelated to this PR specifically, but I was sure that we must have a space before … symbol.

I think only if its within a sentence representing omitted text, when its on a single word there should be no space.

@Antreesy
Copy link
Contributor

If you trying to shorten words as i18n, or cut by the max length - it's an ellipsized text, which shouldn't be spaced

Otherwise it has different meanin. Ellipsis should IMO be spaced from both sides, if it represents:

  • omitted words
  • pause / waiting

@ShGKme
Copy link
Contributor

ShGKme commented Jul 31, 2025

If you trying to shorten words as i18n, or cut by the max length - it's an ellipsized text, which shouldn't be spaced

Which is never the case for l10n string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants