These fail silently at the moment, which is a pain.

Probably need to throw an exception so both the UI and drush can react accordingly.

CommentFileSizeAuthor
#8 module_builder-1662726.patch644 bytesRobin Millette

Comments

Robin Millette’s picture

Robin Millette commented

You mean as in #1661370: hook_groups.template gives json_decode error ?

joachim’s picture

joachim commented

Yup!

I spent ten minutes chasing the same problem because I was on another machine and I hadn't noticed my git pull hasn't worked... :/

Given that the json template files are meant to be overridden by users, having it just fail silently is rubbish. The UI and drush should both complain about it.

oscardax’s picture

oscardax commented

I'm getting this fatal error when accessing admin/modules/module_builder

Fatal error: Call to undefined function json_last_error() in /home/xxxx/public_html/sites/all/modules/module_builder/includes/process.inc on line 178

It seems to be related to this issue thread.
May there be any dependencies for this module not stated in the documentation?

Thanks!!

Robin Millette’s picture

Robin Millette commented

You're right, json_last_error() depends on php >= 5.3.
Might be best to simply check if json_decode() returns NULL to detect errors.

joachim’s picture

joachim commented

Eh crap.

JSON is just rubbish, really isn't it.

Since D8 configuration is now going to be YAML we could switch to that perhaps?

Robin Millette’s picture

Robin Millette commented

I don't use your module enough to really comment, but my gut is telling me to keep json support in 7.2.x and if you really want it, yaml in 7.3.x and 8.3.x (whenever ;-).

Supporting json shouldn't be too hard and it's included in php since 5.2. yaml on the other hand, you have to install separately.

But like I said, I've never touched the template stuff in Module Builder, so I'm not the best person to comment.

I can submit a patch to replace json_last_error() with a test for NULL on json_decode(). Let me know.

joachim’s picture

joachim commented

> yaml on the other hand, you have to install separately

Good point.

> I can submit a patch to replace json_last_error() with a test for NULL on json_decode(). Let me know.

If you have time, yes please!

Robin Millette’s picture

Robin Millette commented
Status: Active » Needs review
StatusFileSize
new module_builder-1662726.patch644 bytes

This patch displays (drupal_set_message) an error if there is a problem with the json file. Not using json_last_error() so it should work with PHP >= 5.2.

joachim’s picture

joachim commented
Status: Needs review » Fixed

Thanks for the patch! Committed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.