backdrop-contrib / tocbot

Builds a table of contents (TOC) from content headings.
GNU General Public License v2.0
2 stars 2 forks source link

Upon reinstall - there are no default settings #22

Closed stpaultim closed 4 years ago

stpaultim commented 4 years ago

I wanted to reset the default settings, so disabled and then uninstalled the module. When I reinstalled it, there were no settings on config page. All empty.

Tocbot___toc

olafgrabienski commented 4 years ago

Oh! As a workaround for the moment, can you delete the tocbot configuration file and try to install again? I’ll have a look at the issue tomorrow.

stpaultim commented 4 years ago

Sure, I got past this by simply destroying my Lando install (for testing) and starting over. I wanted to be safe that I had a clean install.

I recreated the steps and experienced the same problem. During "uninstall" process the configuration is all/mostly removed from the config file, but the config file is not removed. This means that during reinstall, there is already an empty config file for TOCbot.

You can work around this by deleting the existing config file before reinstallation.

olafgrabienski commented 4 years ago

During "uninstall" process the configuration is all/mostly removed from the config file, but the config file is not removed. This means that during reinstall, there is already an empty config file for TOCbot.

I've found a related core issue (https://github.com/backdrop/backdrop-issues/issues/4186) which says that using config_clear in an uninstall function only clears configuration entries in the config file but not the file itself.

In tocbot.install, I use such an uninstall function:

function tocbot_uninstall() {
  config_clear('tocbot.settings', 'tocbot_extrabodyclass');
(...)
}

Now, I'm not sure what I should do instead. There are two solutions / workarounds mentioned in https://github.com/backdrop/backdrop-issues/issues/4186:

(1) For the time being, use config->delete(). Example:

function $mymodule_uninstall() {
  config('mymodule.settings')->delete();
}

(2) Implement hook_config_info(), cf. https://github.com/backdrop/backdrop-issues/issues/4186#issuecomment-548932761.

Which approach is better? If the latter: Could someone help me how to / where to implement hook_config_info()? Due to little coding experience, I've no idea.

olafgrabienski commented 4 years ago

Okay, hook_config_info() seems to be the way to go. Actually, that function is already in tocbot.module, but currenlty there is one important line missing - maybe a Coder Upgrade issue, see https://github.com/backdrop/backdrop-issues/issues/4186#issuecomment-562168627. I'll add however the missing line.

Also, IIRC the function tocbot_uninstall isn't needed in the install file when hook_config_info() is implemented in the module file. See this clarification by @klonos: https://github.com/backdrop/backdrop-issues/issues/4186#issuecomment-548932761. So, I also will remove the uninstall part from the install file.