search

kas-catholic / confessit-web

Source code for https://confessit.app
MIT License
18 stars 7 forks source link

Add Brazilian Portuguese translation #71

Closed marchiartur closed 10 months ago

marchiartur commented 10 months ago

Closing #70

netlify[bot] commented 10 months ago

Deploy Preview for confessit-web ready!

Name Link
Latest commit b1cc6c1d48dda3dabebd3bcd568819a02f1005c2
Latest deploy log https://app.netlify.com/sites/confessit-web/deploys/648dbfe84555720008900e2b
Deploy Preview https://deploy-preview-71--confessit-web.netlify.app/
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

JohnRDOrazio commented 10 months ago

Great job Artur, you did the whole translation in one day! Just a note, you don't need to include the translation in a PR, this is taken care of automatically by Weblate (see pr #69 , which already includes your new Portuguese translation).

kas-catholic commented 10 months ago

@JohnRDOrazio Will it cause problems if I merge this as-is? Or will weblate be able to resolve the changes?

JohnRDOrazio commented 10 months ago

I'm sure that weblate will be able to resolve the merge, I don't believe there are any differences in the translation files.

JohnRDOrazio commented 10 months ago

oh actually I just noticed one thing: Weblate has the language set as Portuguese without any specification of Portugal or Brazil. So the translation file is being saved by weblate in the pt path. Instead, in this PR, it is being pulled as "Brazilian Portuguese" in the ptBR path. So we'll wind up with the same translation twice, under two different paths...

JohnRDOrazio commented 10 months ago

@marchiartur would the Brazilian portuguese be much different from the Portugal portuguese? Perhaps it can be pulled under the pt path, and the pt language added to i18next-scanner.config.js and to src/i18n.js, while you could still keep the BR flag in the language dropdown (but with a value of pt)?

JohnRDOrazio commented 10 months ago

If you think that Portugal Portuguese would be different enough to have it's own translation, we should probably set the language to "Brazilian Portuguese" in Weblate...

JohnRDOrazio commented 10 months ago

Also I think that the general schema for i18next would use pt-BR with a dash, this should ensure the correct language is loaded for any interface elements defined by i18next itself (if there are any).

See:

marchiartur commented 10 months ago

Great job Artur, you did the whole translation in one day! Just a note, you don't need to include the translation in a PR, this is taken care of automatically by Weblate (see pr #69 , which already includes your new Portuguese translation).

Thank you Father John and sorry about included the file in the PR, I didn't perceived this automation. Awesome automation.

marchiartur commented 10 months ago

@marchiartur would the Brazilian portuguese be much different from the Portugal portuguese? Perhaps it can be pulled under the pt path, and the pt language added to i18next-scanner.config.js and to src/i18n.js, while you could still keep the BR flag in the language dropdown (but with a value of pt)?

No, it's not so much different, but probably there are some words more common that are used in Portugal, we can continue with pt-BR or pt translation without problems, because it's understandable for Portugal people and if some Portuguese would like to contribute, I can help with real pt translation.

I've created Brazilian Portuguese in Weblate, but the file was created as pt_BR, do you know if I can configure this folder name (rename it to pt-BR)?

JohnRDOrazio commented 10 months ago

I've created Brazilian Portuguese in Weblate, but the file was created as pt_BR, do you know if I can configure this folder name (rename it to pt-BR)?

Ok I'm seeing this, let me check the weblate settings

JohnRDOrazio commented 10 months ago

After some poking around in weblate, it seems to me that this is an actual bug in Weblate. So I have opened an issue for it:

https://github.com/WeblateOrg/weblate/issues/9412

Weblate does have a few bugs here and there, but has come a long ways and is constantly maintained. I even curated a PR for weblate a year or two ago. Let's wait and see what nijel says about it...

I don't think we should manually change the underscore to a dash, because then it will just be overwritten as soon as any change is made in the translations.

If we want an immediate solution, I would say let's just stick with the generic pt for now (it's already included in the Weblate PR #69), so add pt as the new language value everywhere. You could still show BR in the language dropdown, but just keep pt as the value to make sure it picks up the pt translation file.

marchiartur commented 10 months ago

After some poking around in weblate, it seems to me that this is an actual bug in Weblate. So I have opened an issue for it:

WeblateOrg/weblate#9412

Weblate does have a few bugs here and there, but has come a long ways and is constantly maintained. I even curated a PR for weblate a year or two ago. Let's wait and see what nijel says about it...

I don't think we should manually change the underscore to a dash, because then it will just be overwritten as soon as any change is made in the translations.

If we want an immediate solution, I would say let's just stick with the generic pt for now (it's already included in the Weblate PR #69), so add pt as the new language value everywhere. You could still show BR in the language dropdown, but just keep pt as the value to make sure it picks up the pt translation file.

Ok, perfect, I renamed it.

JohnRDOrazio commented 10 months ago

Nijel was very quick in patching this! https://github.com/WeblateOrg/weblate/issues/9412#issuecomment-1592754392

I'll test the patch tomorrow, today I'm a bit busy.

In any case I would say that we could pull this as is with the latest patch, seeing the patch on weblate will only come out in a new release.

JohnRDOrazio commented 10 months ago

I was able to apply Nijel's patch to my weblate instance today, and I tested adding a new language Spanish (Argentina). It was correctly created as es-AR rather than es_AR.

So I proceeded to remove the Brazilian Portuguese translation, and re-create it. I then added the previous translations one by one using the Automatic suggestions in weblate, so it should have applied all of the previous translations by @marchiartur . This time it was correctly created as pt-BR rather than pt_BR.

I'll keep Nijel's patch in place and avoid updating weblate until a version is issued that includes the patch (should be in the next version in any case).

Basically now we can either pull the current PR as is (it will use the pt translation), or you could revert to using the pt-BR translation. Either should work now (once PR #69 is pulled).

kas-catholic commented 10 months ago

Thanks for the contributions! Should be live now!