When working on a multi lingual page, the correct translated Terms will not get pulled. Neither the term names.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | enable_glossary-2831417-8.patch | 3.51 KB | tassilogroeper |
When working on a multi lingual page, the correct translated Terms will not get pulled. Neither the term names.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | enable_glossary-2831417-8.patch | 3.51 KB | tassilogroeper |
Comments
Comment #2
tassilogroeper commentedPlease test with non multilingual environment
Comment #3
tassilogroeper commentedComment #4
tassilogroeper commentedbtw. the first bock
/onomasticon/src/Plugin/Filter/FilterOnomasticon.php:213-242can be refactored, since the defaults for$this->settingsare always set. see #2831423: Refactoring defaultsComment #5
tassilogroeper commentedadded early return if there is no translation for the current term. No false language will appear in the tooltips. sorry no interdiff.
Comment #6
broonI decided to not load the full entities on purpose. Imagine having a quite large glossary vocabulary, this will result in high memory consumption. So, I tried to fetch a full term entity only if the term name is found in text. In fact, on my test page, I ran out of memory despite having increased the limit to 1024M. And there is only a simple set of terms.
I am aware that
->has/getTranslation()won't work with the partial term objects, though.This would potentially disable all terms that have no language set.
Comment #7
broonComment #8
tassilogroeper commentedpatch #5won't apply, thus forget about it.in this new patch I tested with a multilingual and a non multilingual environment. Unfortunately, I found no other way to enable translation than actually loading the hole
Term- instead of astdClassas right now.I also found a memory leek:
Since the
FilterOnomasticon::process()will get called multiple times, there is a limitation to use a private singleton with the fetched Terms. But if you "cache" at least the recursive calls ofFilterOnomasticon::processChildren(), there is a better memory usage now. In one environment I counted 20 calls without and only 5 with the singleton.btw. the
$this->termCachewould only need the key to be set. no value needed here.Since I did not said it before: very good progress! Really like this module. Thank you for the nice work!
Comment #9
broon$this->termCachegets values because in the latest dev version the user can choose whether to only replace the first occurrence of a term (default, indeed no value needed) or to replace all occurrences (in this case the value is taken from termCache and not loaded again from database.Regarding the translation, I was looking into that as well and neither found any way to just get the available translations without fetching the complete Term objects. We'll need some smoother way to handle this. I just started Drupal 8 a few weeks ago after almost eight years of procedural D6 and D7 and still need to dig through the possibilities of the new system. Also, last time I used Symfony in 2009, it was in major version 1, so I have a pretty steep learning curve right now.
That said, thanks for your kind words and also thank you for the patches provided and all the input. I really appreciate it and it helps me to find my way through the D8 jungle.
Comment #10
broonJust had a quick look at the new patch. Since you created the new
$termsclass variable, my remark about termCache is of course invalid and you are right, now it doesn't need the values.I am unlikely to test it all until Monday due to some deadlines I have to meet, but I will try to do it as soon as possibly.
Comment #12
broonCouldn't sleep well, so I actually did test it. Very good job, it seems to work quite well for Latin-based alphabets. I was tinkering around the last hours trying to get it work for Russian, but failed so far. But in general, the pure translation stuff works.
Comment #13
tassilogroeper commented@paul you are welcome :-)