Skip to content
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

fix: Fix translate substitution when OC is not defined #543

Closed
wants to merge 1 commit into from

Conversation

skjnldsv
Copy link
Contributor

@skjnldsv skjnldsv commented Jan 5, 2023

So, I'm not too sure about this. Here is the context

Context

When testing, we should avoid mocking unnecessary methods and reflect the real execution.
Translate do not just return the string, even if there is no translations for the requested language, it still applies the substitution on the original string.

Concerns

Shall we just move the server code here? Was there a reason for it to call OC instead of shipping working code out of the box? :)

What do you think?

Refs

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv skjnldsv added the type: bug 🐛 Something isn't working label Jan 5, 2023
@skjnldsv skjnldsv self-assigned this Jan 5, 2023
@skjnldsv
Copy link
Contributor Author

skjnldsv commented Jan 5, 2023

Aaand, I just realised @susnux did this 2 days ago
#542

What a timing!

Comment on lines +46 to +47
const r = vars[b]
return r
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const r = vars[b]
return r
return vars[b]

if (typeof OC === 'undefined') {
console.warn('No OC found')
return textSingular
.replace(/%n/g, count?.toString() || '')
.replace(/{([^{}]*)}/g, function(a, b) {
const r = vars[b]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const r = vars[b]
return vars[b]

@skjnldsv
Copy link
Contributor Author

skjnldsv commented Jan 5, 2023

#542

@skjnldsv skjnldsv closed this Jan 5, 2023
@skjnldsv skjnldsv deleted the fix/OC-less-translate branch January 5, 2023 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants