From 15a169ea8af5677392a46cf8a5e07bd9725e431e Mon Sep 17 00:00:00 2001 From: Humorous Baby Date: Fri, 12 Apr 2019 09:35:22 -0400 Subject: [PATCH 1/4] admin: refactor; pull out set command argument parsing --- sopel/modules/admin.py | 92 +++++++++++++++++++++++++++++------------- 1 file changed, 65 insertions(+), 27 deletions(-) diff --git a/sopel/modules/admin.py b/sopel/modules/admin.py index 7f66d285c2..35eb1e4188 100644 --- a/sopel/modules/admin.py +++ b/sopel/modules/admin.py @@ -44,6 +44,19 @@ def setup(bot): bot.config.define_section('admin', AdminSection) +class InvalidSection(Exception): + def __init__(self, section): + super(InvalidSection, self).__init__(self, 'Section [{}] does not exist.'.format(section)) + self.section = section + + +class InvalidSectionOption(Exception): + def __init__(self, section, option): + super(InvalidSectionOption, self).__init__(self, 'Section [{}] does not have option \'{}\'.'.format(section, option)) + self.section = section + self.option = option + + def _get_config_channels(channels): """List""" for channel_info in channels: @@ -245,52 +258,78 @@ def mode(bot, trigger): bot.write(('MODE', bot.nick + ' ' + mode)) -@sopel.module.require_privmsg("This command only works as a private message.") -@sopel.module.require_admin("This command requires admin privileges.") -@sopel.module.commands('set') -@sopel.module.example('.set core.owner Me') -def set_config(bot, trigger): - """See and modify values of Sopel's config object. +def parse_section_option_value(config, trigger): + """Parse trigger for set/unset to get relevant config elements. - Trigger args: - arg1 - section and option, in the form "section.option" - arg2 - value + :param config: Sopel's config + :param trigger: IRC line trigger + :return: A tuple with ``(section, section_name, static_sec, option, value)`` + :raises InvalidSection: section does not exist + :raises InvalidSectionOption: option does not exist for section - If there is no section, section will default to "core". - If value is None, the option will be deleted. + The ``value`` is optional and can be returned as ``None`` if omitted from command. """ - # Get section and option from first argument. match = trigger.group(3) if match is None: - bot.reply("Usage: .set section.option value") - return + raise ValueError # Invalid command + + # Get section and option from first argument. arg1 = match.split('.') if len(arg1) == 1: section_name, option = "core", arg1[0] elif len(arg1) == 2: section_name, option = arg1 else: - bot.reply("Usage: .set section.option value") - return - section = getattr(bot.config, section_name) + raise ValueError # invalid command format + + section = getattr(config, section_name, False) + if not section: + raise InvalidSection(section_name) static_sec = isinstance(section, StaticSection) if static_sec and not hasattr(section, option): - bot.say('[{}] section has no option {}.'.format(section_name, option)) - return + raise InvalidSectionOption(section_name, option) # Option not found in section + + if not static_sec and not config.parser.has_option(section_name, option): + raise InvalidSectionOption(section_name, option) # Option not found in section delim = trigger.group(2).find(' ') # Skip preceding whitespaces, if any. while delim > 0 and delim < len(trigger.group(2)) and trigger.group(2)[delim] == ' ': delim = delim + 1 - # Display current value if no value is given. + value = trigger.group(2)[delim:] if delim == -1 or delim == len(trigger.group(2)): - if not static_sec and bot.config.parser.has_option(section, option): - bot.reply("Option %s.%s does not exist." % (section_name, option)) - return - # Except if the option looks like a password. Censor those to stop them - # from being put on log files. + value = None + + return (section, section_name, static_sec, option, value) + + +@sopel.module.require_privmsg("This command only works as a private message.") +@sopel.module.require_admin("This command requires admin privileges.") +@sopel.module.commands('set') +@sopel.module.example('.set core.owner Me') +def set_config(bot, trigger): + """See and modify values of Sopel's config object. + + Trigger args: + arg1 - section and option, in the form "section.option" + arg2 - value + + If there is no section, section will default to "core". + If value is not provided, the current value will be displayed. + """ + try: + section, section_name, static_sec, option, value = parse_section_option_value(bot.config, trigger) + except ValueError: + bot.reply('Usage: .set section.option [value]') + return + except (InvalidSection, InvalidSectionOption) as exc: + bot.say(exc.args[1]) + return + + # Display current value if no value is given + if not value: if option.endswith("password") or option.endswith("pass"): value = "(password censored)" else: @@ -305,8 +344,7 @@ def set_config(bot, trigger): .format(section_name, option)) return - # Otherwise, set the value to one given as argument 2. - value = trigger.group(2)[delim:] + # Otherwise, set the value to one given if static_sec: descriptor = getattr(section.__class__, option) try: From 659fd471ae4610a157fff4ff21bf48cdd3d67f07 Mon Sep 17 00:00:00 2001 From: Humorous Baby Date: Fri, 12 Apr 2019 09:36:36 -0400 Subject: [PATCH 2/4] admin: add unset command --- sopel/modules/admin.py | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/sopel/modules/admin.py b/sopel/modules/admin.py index 35eb1e4188..dd8af7e3f7 100644 --- a/sopel/modules/admin.py +++ b/sopel/modules/admin.py @@ -322,7 +322,7 @@ def set_config(bot, trigger): try: section, section_name, static_sec, option, value = parse_section_option_value(bot.config, trigger) except ValueError: - bot.reply('Usage: .set section.option [value]') + bot.reply('Usage: {}set section.option [value]'.format(bot.config.core.help_prefix)) return except (InvalidSection, InvalidSectionOption) as exc: bot.say(exc.args[1]) @@ -358,6 +358,37 @@ def set_config(bot, trigger): setattr(section, option, value) +@sopel.module.require_privmsg("This command only works as a private message.") +@sopel.module.require_admin("This command requires admin privileges.") +@sopel.module.commands('unset') +@sopel.module.example('.unset core.owner') +def unset_config(bot, trigger): + """Unset value of Sopel's config object. + + Unsetting a value will reset it to the default specified in the config + definition. + + Trigger args: + arg1 - section and option, in the form "section.option" + + If there is no section, section will default to "core". + """ + try: + section, section_name, static_sec, option, value = parse_section_option_value(bot.config, trigger) + except ValueError: + bot.reply('Usage: {}unset section.option [value]'.format(bot.config.core.help_prefix)) + return + except (InvalidSection, InvalidSectionOption) as exc: + bot.say(exc.args[1]) + return + + if value: + bot.reply('Invalid command; no value should be provided to unset.') + return + + setattr(section, option, None) + + @sopel.module.require_privmsg @sopel.module.require_admin @sopel.module.commands('save') From a86d94462a0c5a25e2ab1375342db96690e590d6 Mon Sep 17 00:00:00 2001 From: Humorous Baby Date: Fri, 12 Apr 2019 09:51:51 -0400 Subject: [PATCH 3/4] admin: prevent `unset`'ing required settings Any setting with `default = NO_DEFAULT` cannot be unset; e.g., `core.owner` --- sopel/config/types.py | 2 ++ sopel/modules/admin.py | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/sopel/config/types.py b/sopel/config/types.py index c2fb876817..6f0ba87e2f 100644 --- a/sopel/config/types.py +++ b/sopel/config/types.py @@ -156,6 +156,8 @@ def __get__(self, instance, owner=None): def __set__(self, instance, value): if value is None: + if self.default == NO_DEFAULT: + raise ValueError('Cannot unset an option with a required value.') instance._parser.remove_option(instance._section_name, self.name) return value = self.serialize(value) diff --git a/sopel/modules/admin.py b/sopel/modules/admin.py index dd8af7e3f7..70eb5228ff 100644 --- a/sopel/modules/admin.py +++ b/sopel/modules/admin.py @@ -386,7 +386,10 @@ def unset_config(bot, trigger): bot.reply('Invalid command; no value should be provided to unset.') return - setattr(section, option, None) + try: + setattr(section, option, None) + except ValueError: + bot.reply('Cannot unset {}.{}; it is a required option.'.format(section_name, option)) @sopel.module.require_privmsg From 01bcf40cf9acf30054bd71c26ef72b7d26ce57d5 Mon Sep 17 00:00:00 2001 From: Humorous Baby Date: Tue, 3 Sep 2019 10:15:50 -0400 Subject: [PATCH 4/4] admin: update set/unset responses to be more informative Previously, there was no feedback from the bot on successful `set`/`unset`. This led to some confusion during testing. Now, the bot will `say` "OK... " upon successful setting/unsetting of an option. Additionally, viewing an option will also show the value `type` to avoid issues like differentiating between `= None (NoneType)` and `= None (str)`, where one may have intentionally set `None` as a `str`, or accidentally set `None` while trying to clear/unset an option. --- sopel/modules/admin.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sopel/modules/admin.py b/sopel/modules/admin.py index 70eb5228ff..9479f95b6e 100644 --- a/sopel/modules/admin.py +++ b/sopel/modules/admin.py @@ -334,7 +334,7 @@ def set_config(bot, trigger): value = "(password censored)" else: value = getattr(section, option) - bot.reply("%s.%s = %s" % (section_name, option, value)) + bot.reply("%s.%s = %s (%s)" % (section_name, option, value, type(value).__name__)) return # Owner-related settings cannot be modified interactively. Any changes to these @@ -356,6 +356,7 @@ def set_config(bot, trigger): bot.say("Can't set attribute: " + str(exc)) return setattr(section, option, value) + bot.say("OK. Set '{}.{}' successfully.".format(section_name, option)) @sopel.module.require_privmsg("This command only works as a private message.") @@ -388,6 +389,7 @@ def unset_config(bot, trigger): try: setattr(section, option, None) + bot.say("OK. Unset '{}.{}' successfully.".format(section_name, option)) except ValueError: bot.reply('Cannot unset {}.{}; it is a required option.'.format(section_name, option))