sopel-irc / sopel

:robot::speech_balloon: An easy-to-use and highly extensible IRC Bot framework. Formerly Willie.
https://sopel.chat
Other
948 stars 402 forks source link

Separate config section definition from bot setup #2597

Open dgw opened 8 months ago

dgw commented 8 months ago

Plugin authors must currently remember to define_section() in two plugin hooks:

  1. setup(), so the plugin's configuration is registered when Sopel runs
  2. configure(), so the plugin's configuration is registered when the interactive config wizard runs

This is a minor inconvenience, true, but it leads directly to a larger problem: Plugin configuration definitions aren't available outside of these two scenarios. If the bot isn't in the config wizard, and isn't running normally, option validation is unavailable.

Implementing command-line sopel-config set functionality (as I started doing a while ago in #1986) becomes tricky, therefore, if we want the CLI to be safe and reject invalid values (such as options not available in a ChoiceAttribute). Plugins' StaticSection definitions have to be available to the CLI, but running a plugin's setup() can come with side effects.

I'm sure we can just setup() plugins before attempting to handle a hypothetical set or unset CLI action, and things would be fine most of the time. I think we can do better long-term, though, and that's what this issue is about: planning for the long term.

In some minor release (hopefully within 8.x), we can add a new way for plugin authors to register their config sections that isn't tied to the plugin's setup() or configure() hooks. It will have to be implemented such that the relevant submodules can do (pseudocode):

if new_method.exists():
    new_method.use()
else:
    old_method()

But assuming we can devise a sensible mechanism, the new and old ways of defining a config section can coexist until the next major Sopel version when we'd remove Config.define_section() from the API.


Considerations

We are theoretically trying to avoid reserving new top-level object names. The simplest way to do so is making any StaticSection subclass auto-loaded, but the catch is that a StaticSection on its own doesn't have a name; the name used in .cfg files comes from the call to Config.define_section().

However, that leaves us with a natural fallback point: Auto-register any StaticSection subclass that has been given a name as part of its definition (using a yet-undetermined mechanism), and leave Config.define_section() to register and name existing subclasses (until the plugin author updates to the new style).

I admit that I haven't had time to think this specific implementation idea all the way through, but it's also just fine to get a gut reaction from the other maintainers when we start on 8.1 development and figure out the path forward together.