-
-
Notifications
You must be signed in to change notification settings - Fork 835
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
Move locale files from language pack to core #2408
Move locale files from language pack to core #2408
Conversation
2c1f6b3
to
8f09b74
Compare
content: content | ||
name_singular: singular name | ||
name_plural: plural name | ||
tag_count_primary: number of primary tags |
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.
Is there any way the tag-specific stuff could remain with tags?
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.
I don't think it matters where these strings are located. It may only be a problem if some extensions use these strings and tags extension is not enabled.
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.
As of right now, the only parts of core that are aware that tags even exists are:
- The installer, which enables bundled extensions
- Some examples in comments
I'd like to avoid expanding that to keep the division between core and extensions as strong as possible. I don't remember off the top of my head, but if we have 2 validation.yml
files, one in core, the other in tags, would they be merged by the translator? If so, we should do that; if not, we may want to investigate supporting that.
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.
For extension you need to put these translations in en.yml
together with the rest of translations for this extension. They should be merged in the same as any other translation.
BTW: Are you sure that this is still used? I can't find any occurrences of these keys on https://github.com/flarum/tags.
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.
Yeah that makes sense. Let's go for that if it works!
BTW: Are you sure that this is still used? I can't find any occurrences of these keys on https://github.com/flarum/tags.
Validation translations aren't referenced directly, but rather used in Laravel validation error messages: If a discussion is submitted with too few primary or secondary tags, these error messages will be used.
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.
Hmm, looks like we found a bug!
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.
Written up at https://github.com/flarum/core/issues/2572. Regardless, I think tag's validation translations should live in the tags repo.
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.
I could remove them here, but IMO this should be done in separate PR.
BTW: I'm not sure if we should use validation.attributes.name
keys outside of validation.yml
. It will encourage extensions developers to do the same, and this is straight way to collisions.
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.
I could remove them here, but IMO this should be done in separate PR.
When all extensions were in a single folder, it made sense to have a shared file for validation logic separated out, but since it's being split, tag-specific validation errors and attribute names should be located in the tags repo.
BTW: I'm not sure if we should use validation.attributes.name keys outside of validation.yml. It will encourage extensions developers to do the same, and this is straight way to collisions.
Why would developers need to override that per-extension, and not just reply on the ones from core? I'm envisioning tag's validation translations being just those 2 attribute names.
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.
When all extensions were in a single folder, it made sense to have a shared file for validation logic separated out, but since it's being split, tag-specific validation errors and attribute names should be located in the tags repo.
I'm not sure how do you want to proceed with this. As I said, I can remove these keys in this PR. But I don't want to block this PR and wait another X months until you decide where these lines should be and what keys they should use (especially as they don't work anyway). Merging this as is and having 2 unnecessary keys should not be a big deal - it could be fixed later as part of #2572.
Why would developers need to override that per-extension, and not just reply on the ones from core?
I'm not talking about overwriting core translations, I'm talking about collisions as a result of using the same namespace (validation.attributes.
) for attributes names of extensions (because official extensions will do this, everyone can copy this pattern) - key validation.attributes.name
could be used by two separate extensions unaware each other, and they may need to be translated differently depending on context (for example in Polish "name" could be translated either as "imię" or "nazwa"). To avoid this it should be something like flarum-tags.validation.attributes.name
, but I'm not sure how problematic this would be with Laravel validator (it looks like black magic for me :/).
f13f64d
to
d159a9c
Compare
See flarum/lang-english#175