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

[en]: Updated incorrect inline strings #4937

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kohlerdominik
Copy link
Contributor

Seems like inline strings were automatically updated at some point. But a lot of values don't make sense with the applied logic.
Example:
The [Begin Date] must be a date before :date. was translated to inline The field must be a date before :date., but it should be The field must contain a date before :date.

So because a lot of placeholders refer to the value and not the key, if the field should be referenced, most strings need to be updated for something like replacing be to contain:

Simplified example: Value should be XField should contain X


This PR updates all inline strings affected.

@andrey-helldar
Copy link
Member

There is no point in touching the English localization files, as they are updated automatically.

Even if you accept this PR, a script will immediately run to check them and fix everything.

@andrey-helldar
Copy link
Member

Technically we can turn off the automatic update of English localization, but we should only do this if it is guaranteed to be wrong and only a manual update will fix the situation.

Next, let's take the key The :attribute must be at least :length characters and contain at least one number.. You suggest using The string must be... instead of This field must be.... In this case I agree, because these keys are related to password validation, and it can only be a string.

But changing the phrase The given :attribute has appeared in a data leak. to The given string has... can be misleading, because the field may not be a string, but a number, for example.

There's also a suggested translation for “dimensions”: “This image has invalid dimensions.", which could also refer to video files, which would already be an error. Here I would consider what can be done.

Nevertheless, overall, your suggestion looks like a real improvement. The only thing is that I personally don't know English and use online translators, so I can't fully say what is right and what is wrong. In this regard, I urge @Laravel-Lang/laravel-lang. Who knows English well? What do you think?

@andrey-helldar andrey-helldar changed the title Updated incorrect inline strings [en]: Updated incorrect inline strings Nov 9, 2024
@kohlerdominik
Copy link
Contributor Author

Hi @andrey-helldar

Thanks for the real good feedback. I'm not native english speaker either. I tried my best, but I'm sure there's quite some room for improvement. And I'm very glad if someone very proficent in the english language could check this again.

Also, good point about the dimensions, there are probably a few other "edge cases" aswell.

However, I feel like with having a majority of sentences usable, it's way more likely to get some native speaker to use this package and therefore contributing their knowledge. Same for edge cases.

@andrey-helldar
Copy link
Member

I agree. It remains to wait for those who know English well :)

@Laravel-Lang/laravel-lang hi! Can you help us?

@michaelkonstantinou
Copy link
Contributor

I am not a native speaker either, but my work environment is in English, and here are my 2 cents about it

It is true that in many cases, the text “This field” does not make any sense. For instance, “This field must be :size kilobytes”. “Field” just doesn’t make any sense. It does make more sense to say that “The file must be :size kilobytes” as correctly this PR changes

However, too descriptive messages may confuse the user or limit the usage of the translation. For instance, “This string must be…”. First of all, “string” makes more sense if you are a developer. If you are not, this doesn’t sound natural. A person would say “This text must be…” if we are talking about a string.

Also, in many cases this can be restrictive. What if I have a password system that does not rely on a string/text user input? It might not be common… but I don’t think this should be restricted as it doesn’t provide any benefits. I also agree that for some security issues this might be misleading

To summarize, this is what I propose:

  • Yes, we could and perhaps it should be a good idea to manually fix english inline translations as this PR very well does
  • It should be accurate… but not strict
  • So, when we are 100% that we are talking about a file, we could use the word file
  • Replacing “This field” with “This value” doesn’t seem like a bad idea although this doesn’t change much. We can apply this change
  • Using strict words like “This string” or “This number” or “This integer”, in my opinion is too restrictive, can be misleading some times and should be avoided.

As for this PR, if “This string” and other similar instances are changed to “This value” or “This field”, it will be ok in my opinion.

@andrey-helldar
Copy link
Member

andrey-helldar commented Nov 18, 2024

A person would say “This text must be…”

I'll put my 2 cents in here too :)

If for a string we can still say “This text should be”, then, for example, for a field with a password it will be unnatural, and in this case it is more correct to write “This password should be...”. Or the name of the product, for example - “This product should be...”.

For automatic translation cases, I can make it so that when new keys are added, they will automatically change to “This value...” for example, but existing ones will not be affected by this change.

This way we can manually manage the “translations” of the English localization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants