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 translation key apostrophe #4630

Merged
merged 1 commit into from
Nov 2, 2021

Conversation

rrelmy
Copy link
Contributor

@rrelmy rrelmy commented Nov 1, 2021

In some languages the translation key was using a different apostrophe than the text from laravel so it was not translated correctly.

For example in the password reset mail the text is used.

Copy link
Contributor

@ebeauchamps ebeauchamps left a comment

Choose a reason for hiding this comment

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

I personnally disagreee with these changes for the translated strings.

So at least in the French version I will stick with the real typographical "apostrophe" when the translated string (key) is enclosed between " and "

@jasonvarga
Copy link
Member

jasonvarga commented Nov 2, 2021

@ebeauchamps The key needs to have the apostrophe as per this PR. You can see how it's used here. If the key doesn't match the English string exactly, the translation won't display.

You can continue to use whatever apostrophe is appropriate for French in the values, though.

@jasonvarga
Copy link
Member

I was wondering why we would have added the incorrect quote in the first place.
It used to be correct. It was changed in laravel/framework#37709.

I also swear there is/was an issue open where someone had pointed out this line in the email is untranslated. I can't find it. This PR would close that.

Thanks @rrelmy

@jasonvarga jasonvarga merged commit c5b30a8 into statamic:3.2 Nov 2, 2021
@rrelmy
Copy link
Contributor Author

rrelmy commented Nov 3, 2021

@jasonvarga If it used to be correct won't my changes break installations with older Laravel versions?
If this would be the case, shall we add both versions?

You might think about this closed bug #4353

@jasonvarga
Copy link
Member

Yes, good point. We probably should 👍

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