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

Return same string object when no interpolations were made #649

Merged
merged 1 commit into from
Feb 28, 2023

Conversation

fatkodima
Copy link
Contributor

See for the details - rails/rails#47421

Even when there are no interpolations, gsub turns SafeBuffer into a String - https://github.com/rails/rails/blob/6af9cba73db6296ab9aec7dbeae1e193ea69f09f/activesupport/lib/active_support/core_ext/string/output_safety.rb#L169
So we need to preserve the original "string" object.

@fatkodima
Copy link
Contributor Author

Not directly related: But isn't it too costly to allocate a new regexp via Regexp.union(config.interpolation_patterns) each time interpolation is done?

@radar
Copy link
Collaborator

radar commented Feb 24, 2023

Could we store that interpolation regex object so that it’s not recalculated every call? Possibly on the config object itself?

@fatkodima
Copy link
Contributor Author

That is done on the other PR you just commented.

@radar
Copy link
Collaborator

radar commented Feb 28, 2023

Attempted to resolve the conflicts here after the other PR was merged in to master. Hope I did it right 🤞

@fatkodima
Copy link
Contributor Author

Resolved conflicts.

@radar radar merged commit b4cd00d into ruby-i18n:master Feb 28, 2023
@radar
Copy link
Collaborator

radar commented Feb 28, 2023

Thank you!

@fatkodima fatkodima deleted the fix-interpolate-same-object branch February 28, 2023 19:34
@jrafanie
Copy link

FYI, if you're using gettext_i18n_rails, you'll need to update to 1.10.1+ which plays nicer with this change in i18n 1.13.0.

See also grosser/gettext_i18n_rails#196

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants