tanmuhittin / laravel-google-translate

Translate translation files to other languages using google translate or another translation api
MIT License
424 stars 71 forks source link

Support for parameters in strings #5

Closed DanielMalmgren closed 5 years ago

DanielMalmgren commented 5 years ago

Hi. First I'd like to thanks for a very good package! This will save me loads of time! I immediately found some small problems though, so I will create a pair of issues :-)

Firstly, it doesn't seem to handle parameters in strings. I'm talking about words prepended with a colon, according to https://laravel.com/docs/5.8/localization#replacing-parameters-in-translation-strings. It seems it simply translates the variable names.

tanmuhittin commented 5 years ago

Related to #4 . There will be a fix commit soon :+1:

DanielMalmgren commented 5 years ago

Oh, sorry. Should have looked more first :-)

tanmuhittin commented 5 years ago

No problem :) Could you verify that fix works using dev-master 👍

DanielMalmgren commented 5 years ago

Doh. Hit another problem now so I'm currently unable to verify. Filed a new issue (#7 ) about it.

DanielMalmgren commented 5 years ago

Ok, it works with some parameters now, not 100% correct though. Examples (all with json translation from swedish to spanish):

"Tidpunkterna mÄste vara i formatet hh:mm!": "Los tiempos deben estar en el formato hh: mm!", Seems it now get confused over this colon which is not a variable.

":name har redan attesterat denna mÄnad!": ": nombre ya certificado este mes!", I guess the problem here is the variable being in the beginning of the string?

"Detta krockar med en registrering som :name har gjort mellan klockan :from och :to samma dag!": "ÂĄEsto se bloquea con un registro que :name ha hecho entre :from y : para marcar ese dĂ­a!", The two first variables here is alright, don't really know what happened to the last one?

"Grattis, du hade rÀtt pÄ :percent% av frÄgorna pÄ första försöket!": "¥Felicidades, tenías razón en el porcentaje de preguntas :percent en el primer intento!", "Du hade rÀtt pÄ :percent% av frÄgorna pÄ första försöket!": "¥Tenías razón en :percent% de las preguntas en el primer intento!", This is kinda weird. It made the second one completely correct, but in the first one it lost the % sign?

"(Ange :alternatives alternativ)": "(Especifique alternativas a las alternativas de :)", Again, for some reason lost this variable.

Except for the examples above everything worked like a charm. So I could simply fix them by hand, but I guess you want to dig into the problem? :-)

tanmuhittin commented 5 years ago

Problem is that Laravel knows all the parameter keys while replacing them : https://github.com/laravel/framework/blob/ada35036792e7565b1e0cdf30f0fb7da88b41e1f/src/Illuminate/Translation/Translator.php#L258

But we need a rule to detect the parameters. Here is my solution : https://github.com/tanmuhittin/laravel-google-translate/blob/699a9bf8d32f80459b0605b92b3f1b6b32abe06c/src/commands/TranslateFilesCommand.php#L126

Which means match anything which starts with space or : AND is followed by : AND is followed by non space or non : character. But looks like it needs improvements.

Using your example texts I created another regex. Please check if it works fine using https://regex101.com/r/PfhQDR/1

DanielMalmgren commented 5 years ago

Yep, that's much better, it detects all my parameters perfectly now. Some small problems persists (examples now with translation from swedish to danish 😉 ):

"Tidpunkterna mÄste vara i formatet hh:mm!": "Timingen skal vÊre i formatet hh: mm!", For some reason it introduces a space after the colon. Guess it is kinda off topic for this issue, but still irritating. (edit: Just realized that this seems to be Googles fault, so I guess it's not much to do anything about)

"(Ange :alternatives alternativ)": "(Angiv alternativer til :-alternativer)" Not really sure what happened here. According to the test on regex101 it finds the parameter fine enough but then it does something strange with it. I think the translation should end up with something like "(Angiv :alternatives alternativer)".

tanmuhittin commented 5 years ago

Could not find a way to tell Google Translate "Do not translate this piece of text". Proposed solutions like notranslate class or others do not work properly. Currently the master is replacing ":" with "PIKACHU" so that it can replace them back after translation. Using your example texts I did some trials and it is also not a good way doing that.

tanmuhittin commented 5 years ago

Here is your texts translated using last commit:

{
"Tidpunkterna mÄste vara i formatet hh:mm! " : "Los tiempos deben estar en el formato hh: mm!", #this is because of Google Translate. Could not fix that
":name har redan attesterat denna mÄnad!" : ":name ya ha certificado este mes!",
"Detta krockar med en registrering som :name har gjort mellan klockan :from och :to samma dag!" : "ÂĄEsto se bloquea con un registro que :name ha hecho entre :from y :to el mismo dĂ­a!",
"Grattis, du hade rÀtt pÄ :percent% av frÄgorna pÄ första försöket!":"¥Felicidades, tenías razón en el :percent% de las preguntas en el primer intento!",
"(Ange :alternatives alternativ)" : "(Entre las opciones de :alternatives)"

}

Please check that everything is fine using dev-master 👍

DanielMalmgren commented 5 years ago

Yep. Got 'em all right now. Nice work!