roryg / ElementHelper

Element Helper is a MODx Revolution plugin for automatically creating elements from static files without the MODx manager.
27 stars 10 forks source link

Update ElementHelper to version 1.4.0, see changelog/code for details #16

Closed exside closed 9 years ago

exside commented 11 years ago

Sorry for that messy / big PR, but I changed too much in the last months to break them into single ones, I also added a ready to use package (which is not the very newest bc I don't know how to replace a file in a commit...), so you can test easier. For me it seems to work great, I just didn't test the TV creation stuff, as I never use it...so you may have an eye on it if everything works as it should with TVs.

exside commented 11 years ago

oh man...github is still catching me all the time...just discard the commits 1761772 and eacca5a

exside commented 11 years ago

please wait with a possible merge! I have a veeeery strange issue with wayfinder not outputting any submenus anymore, no idea why, still debugging like crazy...made a completely new setup, installed only wayfinder, pasted in the same chunks and it worked without problems, then installed elementhelper 1.4.0 and it suddenly doesn't output any subnavs anymore...deactivated the plugin but still no subnavs, so could not make them come back yet, that needs to be fixed before any release! If somebody has an idea wtf this could cause, I'm very thankful for hints =)

exside commented 11 years ago

OK, I'm getting closer...seems to be related to the source of the static element...if it's set to 0 it works and if it's set to anything else it doesn't work anymore, but why...no idea, in wayfinder the chunk is correctly fetched, but then magically the [[+wf.wrapper]] placeholder disappears when the chunk is processed^^...really...wtf

UPDATE: Actually the same thing happens with ElementHelper 1.3.2...so none of my changes seem to be the problem...maybe it's a wayfinder/modx bug, I'm using 2.2.9pl traditional and advanced (same effect) with wayfinder 2.3.3pl, diggin' further

UPDATE: Same problem with wayfinder 2.3.2pl

UPDATE: Problem not existent with 2.2.8pl traditional...seems to be a 2.2.9 issue...shit...will file a bug in the tracker, no idea where this is coming from...maybe something changed with the parsing process? Eventually has to do with changes like:

UPDATE: After manually reverting the above bugs it's clear that the guilty one is:

when I use the old parser logic fron 2.2.8 in a 2.2.9 installation, everything works fine as before...gonna file this tomorrow, it's late =)

UPDATE: Filed a bug in the tracker: http://bugs.modx.com/issues/10211

UPDATE: There is also a forum thread: http://forums.modx.com/thread/86431/nested-snippet-call-in-rowtpl-wayfinder-work-inproperly-after-update-to-modx-2-2-9#dis-post-478276

Jason recommends there to NOT use media sources with static files... @roryg: do you remember why we implemented this in the first case? =) (I think I did, but why...?)

roryg commented 11 years ago

Hi exside,

Thanks for all this work but unfortunately it's just too big for me to properly review and merge. To make the repo easier for me to manage it would be best if you created a separate branch for each feature/bug fix and then sent a pull request for each one.

I think it must have been you that implemented the media sources for static files option. I've always just used the default 'Filesystem' media source so I'm not sure what the original reason was...

If you want to just continue working on your fork separately I'd be happy to add a link to it in the read me and on the page in the package manager. :)

exside commented 11 years ago

I know its a mess, but I'm using it on all my installations of elementhelper (around 20), and it works nicely, so nothing will break (except the 2.2.9 parser issue, which was fixed today and should work properly with 2.2.10 again (have to test that tough)...it's very time consuming to make separate prs for all the changes as I (stupidly) worked for a longer time on elementhelper without pushing to my repo..., maybe you can try the provided package from my fork and see if it also works for you and then maybe again think about merging it...I don't think it makes a lot of sense to have to repos for elementhelper =/

I actually think the media source thing was because of an issue with MIGX (which couldn't work with anything else than media source 1 but editors shouldn't have access to the elements folder...

exside commented 11 years ago

nothing against PSR-2, just a lot of work to change everything =), but will consider if I find time to make this PR less messy by splitting it up in different PRs/Branches...