-
Notifications
You must be signed in to change notification settings - Fork 111
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 allows i18nTextCollector to concatenate keys #824
FIX allows i18nTextCollector to concatenate keys #824
Conversation
fixes tractorcow-farm#823 - The magic constant __TRAIT__ was inserting namespaces into localization keys, containing slashes that could not be parsed by the TextCollector task. These have been replaced w/ the Trait name as a string
I wasn't 100% sure which branch to submit a PR to patch version 6. Let me know if this belongs on another branch. |
That's not actually correct, textcollector has always been able to support namespaces with slashes in them. Here is the currently extracted strings. Are you running an older or outdated version of text collector? TractorCow\Fluent\Extension\Traits\FluentAdminTrait:
ClearAllNotice: 'All localisations have been cleared for ''{title}''.'
CopyNotice: 'Copied ''{title}'' to all other locales.'
DeleteNotice: 'Deleted ''{title}'' and all of its localisations.'
UnpublishNotice: 'Unpublished ''{title}'' from all locales.'
ArchiveNotice: 'Archived ''{title}'' and all of its localisations.'
PublishNotice: 'Published ''{title}'' across all locales.' |
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.
To investigate possible regression in text collector
It could also matter maybe what PHP version you're testing it on. |
Yeah, I after a bit more digging it does appear to not be about the namespaces but something about the __TRAIT__ magic constant itself. Passing the slashes in as a string works, but not w/ __TRAIT__ I'm running Silverstripe Framework 4.13.29 on PHP 8.1 |
Ok, we should be fine to replace |
Yes, I was coming to the conclusion that was the correct solution already but hadn't yet updated the PR. Thanks. |
Strings updated to include full namespace. Presumably this change needs doing on Fluent v7 as well but I am not running Silverstripe 5 and haven't tested it. |
I honestly think this is being addressed in the wrong place - the textcollector should instead be updated to correctly handle |
Can be merged when tests pass. |
@GuySartorelli It looks like this may have been fixed in framework 5 but not applied to 4? silverstripe/silverstripe-framework@f4e0c76 |
@davejtoews oh, nice find! |
Description
fixes #823 - The magic constant TRAIT was inserting namespaces into localization keys, containing slashes that could not be parsed by the TextCollector task. These have been replaced w/ the Trait name as a string
Manual testing steps
Run
/dev/tasks/i18nTextCollectorTask
with the environment set to Dev or error reporting otherwise configured to display warnings.Issues
#823