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

Update FR translations #52

Merged
merged 2 commits into from
Jun 24, 2024

Conversation

damsfx
Copy link
Contributor

@damsfx damsfx commented Jun 18, 2024

  • Add missing translations
  • Apostrophe standardization

- Add missing translations
- Apostrophe standardization
@LukeTowers
Copy link
Member

@mjauvin thoughts on the apostrophe suggestion in this PR?

@mjauvin
Copy link
Member

mjauvin commented Jun 19, 2024

@mjauvin thoughts on the apostrophe suggestion in this PR?

@damsfx I don't really like this, we use standard single quote everywhere else, you just need to use double quotes around the whole string.

@bennothommo
Copy link
Member

I also believe that some OS's (or some fonts that those OS's use) may also substitute single quotes with more "fancy" versions as needed. It's likely that won't happen if you are already using a "fancy" quote.

@damsfx
Copy link
Contributor Author

damsfx commented Jun 19, 2024

@LukeTowers @mjauvin @bennothommo
In French, we make the difference between apostrophe ( ’ ) and prime ( ' ) symbols.

  • The prime symbol is used it in mathematics and measurements ( e.g. 6'7'' ).
  • The apostrophe is a typographical sign for grammatical connections (e.g. Marc’s computer ).

Having said that, I don't have problem following the WinterCMS convention on this subject, I just wanted to standardize the translations because both forms are used in the same file.
My choice was based on habit.

@mjauvin
Copy link
Member

mjauvin commented Jun 19, 2024

@damsfx I would prefer if you don't escape the single quote and use double quote around the whole strings when a single quote is used inside

@mjauvin
Copy link
Member

mjauvin commented Jun 19, 2024

Unless the other maintainers prefer the escaped single quote?
@LukeTowers @bennothommo @jaxwilko

@jaxwilko
Copy link
Member

I prefer single quotes with escapes as it prevents you accidently parsing a var when you are intending to display a variable name. I.e.

echo 'Hello, I\'m sure you meant to set $example';
// vs.
echo "Hello, I'm sure you meant to set $example";

I'm not massively concerned either way though :)

@LukeTowers
Copy link
Member

I don't have an opinion personally, although I'd prefer to more closely follow the style conventions of the language in question so that it's a more natural experience for users of that language. Having said that, I don't actually use French so I'll defer to the people who actually use it to provide recommendations on what we use for punctuation in our localization strings.

@bennothommo
Copy link
Member

bennothommo commented Jun 24, 2024

I prefer single quotes and escaping too. Not only does it more evidently separate the actual string from variables and the like, I still have this unproven crackpot theory that favouring these types of strings provides a really marginal performance improvement because PHP doesn't have to parse the strings for variables.

EDIT: Also, happy to take @damsfx's advice on the quotes with respect to using the special quotes depending on the context.

@mjauvin
Copy link
Member

mjauvin commented Jun 24, 2024

Allright, we're going to use single quotes and escape the single quote when present. We should do this accross the code base for consistency.

Thanks @damsfx

@mjauvin mjauvin self-assigned this Jun 24, 2024
@mjauvin mjauvin merged commit 6e8223c into wintercms:main Jun 24, 2024
0 of 3 checks passed
@mjauvin mjauvin added this to the v2.2.2 milestone Jun 24, 2024
@LukeTowers
Copy link
Member

@mjauvin Could you add that to the Developer Guide so that we have a published reference for the future?

@mjauvin
Copy link
Member

mjauvin commented Jul 6, 2024

@LukeTowers see wintercms/docs#196

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.

5 participants