config: support quoting ListAttribute values #1689
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Used to make `core.channels` option safe in multi-line mode. Channel names usually start with '#'. ConfigParser interprets any line (after stripping whitespace) that starts with '#' as a comment and ignores it. Thus, when Sopel was told to save its config, serializing the list of channels broke that option and the list would be loaded as empty on the next restart. Quoting each value fixes that. Adding this as a feature of ListAttribute itself, rather than doing the magic elsewhere (like where the channel list is used), made this fix a lot cleaner. It also means that plugins needing to allow '#' in their own ListAttribute values can simply toggle this flag instead of having to implement their own magic. Credits: @Exirel suggested this solution right before going to bed. I decided to implement it while he was sleeping, as a surprise. :D
There's a lot to parse (ha!) here, but most of it is pretty obvious in purpose. The one thing I think I should explain is why all the `parse()` and `serialize()` instances have their own docstrings now. It's because I was having fun! Just kidding (sort of). It's actually because Sphinx autodoc pulls the docstring from the parent class's method if a subclass's overriding method doesn't have a docstring of its own. In hindsight, writing new docstrings might have been *more* work than the alternative (setting `autodoc_inherit_docstrings = False` in `docs/conf.py` and checking to see if anything broke), but what's done is done at this point. We can still change the setting later.
dgw
added
High Priority
Tweak
Documentation
Bugfix
dgw
referenced
this pull request
Sep 8, 2019
You can't get the docstring of an attribute, sadly. Close #864
|
Decided to go with #1690's solution. The doc updates will become a separate PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Likely a better alternative to #1687. Suggested by @Exirel a little while ago as he was heading to bed. I felt like doing something code-y, so here we are.
Most of the time I spent on this actually went into the docstring updates, which are separated into their own commit. That will make it easier to pull them out and (with minor tweaks) apply the same fixes in a separate PR if we ultimately go with a different solution to the
#problem.Of note: I removed a reference to some feature of
StaticSection.configure_setting()that never worked (havingpromptbe an optional argument). See #864 & ab83a1f.