-
Notifications
You must be signed in to change notification settings - Fork 11
FEATURE: Term replacement #62
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
Conversation
… use a different approach to have terms ignored by DeepL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds enhanced term replacement functionality that extends the existing ignored terms feature to support per-language replacement, while remaining backwards compatible. Key changes include:
- Creation of a new ReplaceTermsUtility class along with its tests for replacing terms with their target language translations.
- Removal of the IgnoredTermsUtility and corresponding tests.
- Updates in DeepLTranslationService, configuration, and documentation to support the new term replacement capability.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
Tests/Unit/Utility/ReplaceTermsUtilityTest.php | Introduces tests for validating term replacement data and wrapping/unwrapping behavior. |
Tests/Unit/Utility/IgnoredTermsUtilityTest.php | Removed as the functionality is now replaced by term replacement logic. |
Tests/Unit/Infrastructure/DeepL/DeepLTranslationServiceTest.php | Adjusted tests to expect new tag patterns () for replaced terms. |
README.md | Updated documentation to explain the new term replacement feature and provide an example configuration. |
Configuration/Settings.yaml | Added settings comments and configuration for replaceTerms. |
Classes/Utility/ReplaceTermsUtility.php | Implements term replacement logic while preserving backwards compatibility. |
Classes/Utility/IgnoredTermsUtility.php | Removed in favor of the new utility. |
Classes/Infrastructure/DeepL/DeepLTranslationService.php | Updated to use term replacement wrapping and unwrapping. |
Classes/Domain/ReplaceTerm.php | New domain class encapsulating a term and its translation. |
@gradinarufelix Could you check out the Glossary PR #63 first and tell me afterwards wether you still want this here merged. In my experience the glossaries allow to achieve pretty much the same but can be managed by editors. I would like to prevent adding too much similar features. Also i do not think it is viable to have translations in Settings for any sizable project. It rings my footgun bells. |
I will do that. If it works like my term replacement, I'd be happy to give it up. Our use case was that a customer wanted a certain term in German to always be replaced with the exact term in English. When I did some research, the glossary function did not seem to guarantee that. What were your experiences? Does it replace a term with exactly the term specified in the glossary then? |
In my experience it pretty much worked very well. Glossaries allow some flexibility but DeepL pretty much sticks to it. An advantage is that a glossary entry is better suited for cases where grammar is involved and a 1:1 replacement does not really work well. |
I extended the ignored terms function to also allow replacement of specific terms. All changes were made so they are backwards compatible and no migration of the
ignoredTerms
configuration needs to be done.