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

feat (translation): add ability to ignore keys in translations #199

Merged
merged 4 commits into from Dec 18, 2023

Conversation

kylemilloy
Copy link
Contributor

@kylemilloy kylemilloy commented Dec 16, 2023

What does this change?

Allows us to provide keys to translate without eating them up, for example, Hello :name was becoming Bonjour :nom. Now this will stored as Bonjour :name still.

How does this work?

We do a couple preg_replace commands to extract replacements, modify the string before sending to Google, and then swap back in our replacements. In a string such as Hello :name are you :type_of_greeting? for example this replaces this string with Hello ${0} are you ${1}? with the ${\d} values being ignored by Google, everything around it gets translated. When we receive this translated string back we then swap these values out for the matching keys that should be there so it changes from Bonjour ${0} are you ${1} to Bonjour :name are you :type_of_greeting? where our system can then translate it as normal.

Notes

Resolves #198

Also introduces ambiguous space characters being introduced by Google Translate (see screenshot).

image

image

Copy link
Owner

@Stichoza Stichoza left a comment

Choose a reason for hiding this comment

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

Thanks for your work! 🙌🏼 Couple of changes:

  1. For easier understanding, can you rename methods extract -> extractParameters, inject -> injectParameters, getReplacements -> getParameters.
  2. Make above mentioned methods protected. Also, no need of testing non-public methods.
  3. Make this feature optional. Would be better to get rid of the method setPattern and add preserveParameters(?string $pattern = null) that will enable this feature and set custom pattern if specified.

@Stichoza Stichoza self-assigned this Dec 17, 2023
@kylemilloy
Copy link
Contributor Author

kylemilloy commented Dec 17, 2023

@Stichoza

I've gone ahead and responded to your feedback. I have it so it always runs through the key extraction but it's ignored if you don't set it explicitly as requested. Tests are expanded to show a couple examples of this and one for custom key matching (like if you wanna do {{key}} for example).

You can also set this off of the static method or constructor too for ease of use...

// like this...
$translator = new GoogleTranslate(pattern: '/:(\w+/');

// or this...
GoogleTranslate::trans(pattern: '/:(\w+/');

// or this
$translator->preserveParameters('/:(\w+/');

If patterns like :name and :key are the default I have it so you can just call $translator->preserveParameters() and it will do that for you, or you can customize it as above. In the constructor we pass null here instead to turn it off by default.

Copy link
Owner

@Stichoza Stichoza left a comment

Choose a reason for hiding this comment

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

Great! Thank you very much! 🥳

@Stichoza Stichoza merged commit 9e2e66b into Stichoza:master Dec 18, 2023
8 checks passed
Stichoza added a commit that referenced this pull request Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ignore escaped keys on translations
2 participants