DHTML Menu uses system variables for exactly two purposes: Settings, and backing up part of the theme registry.
The settings are all set on the same system settings form, and even though some of the functions use only one or two of the settings keys, they all have to unserialize the same defaults constant to put a default in the variable_get call.
The two issues are 1) that the variables are all over the "dhtml_menu" variable namespace, and need to be placed into the settings array one by one to send them to Javascript - 2) that each function calling variable_get() has to unserialize the defaults and pass the corresponding default to variable_get whenever it needs a setting.
This change should move all settings variables into one keyed array stored at dhtml_menu_settings. A system_settings_form can easily handle #tree-using fields; these are stored in serialized form under the key of the root field.
Also, the default constant should be removed, in favor of setting the default value with variable_set once when the module is installed.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | dhtml_menu-consolidate-settings-478472-2.patch | 14.13 KB | cburschka |
| #1 | dhtml_menu-consolidate-settings-478472-1.patch | 13.79 KB | cburschka |
Comments
Comment #1
cburschkaHere is an initial patch based on the bigger changeset in #473356: DHTML Menu takes another leap.
It also adds the hook_enable, hook_disable, hook_install and hook_uninstall implementations since that makes sense. An upgrade path is included.
Comment #2
cburschkaFound several bugs in my code. Here's a new version.
Comment #3
cburschkaCommitted to HEAD.
Note: hook_variable_info() may eventually be used here once it is added to core, but I don't want to introduce further D7-specific features into the module when the branch will still be split off for D6 at some point.
This also contains a few code-style muckups, but there will be an all-round clean-up patch for those anyway.