hpolthof / laravel-translations-db

Laravel Translations from Database
https://packagist.org/packages/hpolthof/laravel-translations-db
GNU General Public License v2.0
52 stars 37 forks source link

Illegal string offset 'email' in DumpCommand #13

Closed renege closed 7 years ago

renege commented 8 years ago
ErrorException · vendor/hpolthof/laravel-translations-db/src/Console/Commands/DumpCommand.php:109
Illegal string offset 'email'
renege commented 8 years ago

@hpolthof can't figure out what it is.

dangermark commented 8 years ago

I had a similar issue, traced it down to the key notation.

If you have 2 entries, like the following:

contact = 'Contact Us' contact.email = 'Enter your email'

This will fail, as the dump command tries to create arrays from these keys to generate the language files, and the array keys will conflict. Get around it by using a pattern like:

contact.title = 'Contact Us' contact.email = 'Enter your email'

renege commented 8 years ago

so this is a bug in @hpolthof's package

raphaelsaunier commented 8 years ago

⚠️ Quick word of warning to anyone who plans on using this package. Do not install it and just expect it to work.

We've wasted a lot of developer and translator hours because of the numerous bugs in this package. Translations were lost/overwritten after running translation:fetch, restoring from database backups failed because of broken unique key constraints. In addition to that, the package cannot be disabled entirely on production, where it unpredictably caches the wrong translations.

Regarding the Illegal string offset issue, we were able to salvage the translations by replacing this line in Commands/DumpCommand.php:

$this->assignArrayByPath($new_content, $key, $value);

With:

if($value){
    array_set($new_content, $key, $value);
}
ksort($new_content);
Jaspur commented 8 years ago

@raphaelsaunier send a PR then.....

raphaelsaunier commented 8 years ago

@Jaspur ha, yes of course, that's exactly the kind of things you do when your project is late, to a large extent because of the stress and headaches that this package caused. 😆

Listen, I hate people who complain about stuff they get for free just as much as the next person. All I did was to: 1) Warn prospective users to thoroughly test this package before letting translators use it because there is a data loss risk 2) Suggest a temporary fix for the issue described above

Jaspur commented 8 years ago

@raphaelsaunier you don't get what open source is then. And in the same time you wrote the oh-so-useful warning, you could have submitted a PR, because that's how open source works.

raphaelsaunier commented 8 years ago

@Jaspur Thanks for educating me about how open source works. As mentioned in my first comment, I believe that there are some more fundamental issues with this package that can't be fixed in just a few hours. Moreover, I don't even know if my suggested fix works reliably in all situations. A PR would imply the opposite.

renege commented 8 years ago

@raphaelsaunier seriously dude? make a PR instead of crying out loud.

raphaelsaunier commented 8 years ago

@renege Feel free to take my fix and submit a PR if it solves the problem for you as well!

hpolthof commented 8 years ago

Added the fix to the master code. Please not that you should not assign values on different levels.

so using: contact and contact.email will kill your translations.

hpolthof commented 8 years ago

@Jaspur your fixed kept my projects working ;-)

As this piece of code is written for my own projects and just shared with the OS community, this software comes as-is. As most OS packages I might add.