- First commit to issue fork.
- last update
over 1 year ago 479 pass - 🇯🇵Japan tyler36 Osaka
The functional tests are now passing.
I added the missing schema and made the plugin a little more explicit in its handling of config data. - 🇩🇪Germany jurgenhaas Gottmadingen
Thanks @tyler36 this is looking great. Hope it gets merged eventually.
- last update
over 1 year ago 479 pass - 🇳🇱Netherlands megachriz
Since the string separator is a fixed value (not to be changed for this plugin), I removed it as a setting from the plugin. Instead
getSetting()
gets overridden, so that it returns the separator that the Explode plugin asks for.@jurgenhaas
Do you agree with these changes?I still wonder if it is a good idea that this plugin extends the Explode plugin. I just reviewed ✨ Explode values as keys Needs work that adds a setting to the Explode plugin that makes no sense for the WordCount plugin to have.
- 🇩🇪Germany jurgenhaas Gottmadingen
Thanks @MegaChriz, I've looked at your changes and they really look nice.
I still wonder if it is a good idea that this plugin extends the Explode plugin.
Good question. If it were not extending that, there would be quite a bit of redundant code. On the other hand, I can see your point that further changes to the explode plugin could require extra attention in the word count plugin.
I can't really tell what's the better option, happy to leave that decision to yourself.
- last update
over 1 year ago 479 pass - last update
over 1 year ago 479 pass -
MegaChriz →
committed 55c04dee on 8.x-1.x authored by
jurgenhaas →
Issue #3279973 by jurgenhaas, MegaChriz, ikeigenwijs, tyler36: Added...
-
MegaChriz →
committed 55c04dee on 8.x-1.x authored by
jurgenhaas →
- Status changed to Fixed
over 1 year ago 10:08am 12 September 2023 - 🇳🇱Netherlands megachriz
I made the WordCount plugin to no longer extend the Explode plugin and I merged the code!
Automatically closed - issue fixed for 2 weeks with no activity.