Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

admin: .set cannot clear values #1554

Closed
HumorBaby opened this issue Apr 11, 2019 · 0 comments · Fixed by #1556
Closed

admin: .set cannot clear values #1554

HumorBaby opened this issue Apr 11, 2019 · 0 comments · Fixed by #1556
Labels
Bug Things to squish; generally used for issues
Milestone

Comments

@HumorBaby
Copy link
Contributor

The .set command of admin.py does not work as described.

@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 None, the option will be deleted.
"""

says that if value is None, the option will be deleted. Currently, a None value does not get parsed properly, and the setting just ends up getting converted to a literal None string, as opposed to an actual NoneType.

I considered that the sentence meant that no value should be supplied, but that would preclude the See part of the See and modify values of Sopel's config object, where no argument should be provided for setting (which is working as expected). Thus, 🐛


Steps to replicate:

# admin_set_test.py

from __future__ import unicode_literals, absolute_import, division, print_function

from sopel.config.types import StaticSection, ValidatedAttribute
import sopel.module


class AdminSetTestSection(StaticSection):
    test_setting = ValidatedAttribute('test_setting', default=None)


def setup(bot):
    bot.config.define_section('admin_set_test', AdminSetTestSection)


@sopel.module.require_privmsg
@sopel.module.commands('ast-show')
def show(bot, trigger):
    bot.say('repr: "{}", type: "{}"'.format(
        repr(bot.config.admin_set_test.test_setting),
        type(bot.config.admin_set_test.test_setting)
    ))
    
# my-config.cfg
## Note: there is no `test_setting` set, so it defaults to `None` (`NoneType`)

[admin_set_test]

# _See_ current setting value
<HumorBaby>  .set admin_set_test.test_setting
<testbot>  HumorBaby: admin_set_test.test_setting = None

# See repr and type of setting
<HumorBaby>  .ast-show
<testbot>  repr: "None", type: "<type 'NoneType'>"

# Set a random value to setting
<HumorBaby>  .set admin_set_test.test_setting abc123

# Confirm change
<HumorBaby>  .set admin_set_test.test_setting
<testbot>  HumorBaby: admin_set_test.test_setting = abc123
<HumorBaby>  .ast-show
<testbot>  repr: "u'abc123'", type: "<type 'unicode'>"

# Try to "delete"/unset setting
<HumorBaby>  .set admin_set_test.test_setting None

# Confirm change
<HumorBaby>  .set admin_set_test.test_setting
<testbot>  HumorBaby: admin_set_test.test_setting = None
# everything looks okay...

# But wait... wat?
<HumorBaby>  .ast-show
<testbot>  repr: "u'None'", type: "<type 'unicode'>"

So, I wondered if .save'ing the config would somehow solve the issue...

<HumorBaby>  .save

# Restart bot here...

<HumorBaby>  .set admin_set_test.test_setting
<testbot>  HumorBaby: admin_set_test.test_setting = None
<HumorBaby>  .ast-show
<testbot>  repr: "'None'", type: "<type 'str'>"

Nope...

@HumorBaby HumorBaby added the Bug Things to squish; generally used for issues label Apr 11, 2019
@dgw dgw changed the title .admin set cannot clear values admin: .set cannot clear values Apr 22, 2019
@dgw dgw added this to the 7.0.0 milestone Apr 22, 2019
@dgw dgw closed this as completed in #1556 Sep 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Things to squish; generally used for issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants