config: quote ListAttribute items when required

In a ListAttribute, a value can start with a #, which is then considered
as a comment by the Config reader. This is bad news, as channel names
usually start with a # (they can start with a & too, which is less
common on freenode).

To prevent this, what I do is:

* quote the item only if necessary
* unquote it if it correspond to a very specific pattern `"#<name>"`

This way, we can have this ListAttribute:

    channels =
        "#sopel"
        &private

Edge case like `"&something"` won't be unquoted, because they don't
match the exact pattern!

(with applied suggestions from code review)

Co-Authored-By: dgw <[email protected]>

sopel/config/types.py

@@ -27,6 +27,7 @@
 from __future__ import unicode_literals, absolute_import, print_function, division
 
 import os.path
+import re
 import sys
 from sopel.tools import get_input
 
@@ -221,7 +222,7 @@ class SpamSection(StaticSection):
     the option will be exposed as a Python :class:`list`::
 
         >>> config.spam.cheeses
-        ['camembert', 'cheddar', 'reblochon']
+        ['camembert', 'cheddar', 'reblochon', '#brie']
 
     which comes from this configuration file::
 
@@ -230,6 +231,12 @@ class SpamSection(StaticSection):
             camembert
             cheddar
             reblochon
+            "#brie"
+
+    Note that the ``#brie`` item starts with a ``#``, hence the double quote:
+    without these quotation marks, the config parser would think it's a
+    comment. The quote/unquote is managed automatically by this field, and
+    if and only if it's necessary (see :meth:`parse` and :meth:`serialize`).
 
     .. versionchanged:: 7.0
 
@@ -254,6 +261,13 @@ class SpamSection(StaticSection):
         the list.
     """
     DELIMITER = ','
+    QUOTE_REGEX = re.compile(r'^"(?P<value>#.*)"$')
+    """Regex pattern to match value that requires quotation marks.
+
+    This pattern matches values that start with ``#`` inside quotation marks
+    only: ``"#sopel"`` will match, but ``"sopel"`` won't, and neither will any
+    variant that doesn't conform to this pattern.
+    """
 
     def __init__(self, name, strip=True, default=None):
         default = default or []
@@ -276,33 +290,64 @@ def parse(self, value):
             multi-line string.
         """
         if "\n" in value:
-            items = [
+            items = (
                 # remove trailing comma
                 # because `value,\nother` is valid in Sopel 7.x
                 item.strip(self.DELIMITER).strip()
-                for item in value.splitlines()]
+                for item in value.splitlines())
         else:
             # this behavior will be:
             # - Discouraged in Sopel 7.x (in the documentation)
             # - Deprecated in Sopel 8.x
             # - Removed from Sopel 9.x
             items = value.split(self.DELIMITER)
 
-        value = list(filter(None, items))
-        if self.strip:  # deprecate strip option in Sopel 8.x
-            return [v.strip() for v in value]
-        else:
-            return value
+        items = (self.parse_item(item) for item in items if item)
+        if self.strip:
+            return [item.strip() for item in items]
+
+        return list(items)
+
+    def parse_item(self, item):
+        """Parse one ``item`` from the list.
+
+        :param str item: one item from the list to parse
+        :rtype: str
+
+        If ``item`` matches the :attr:`QUOTE_REGEX` pattern, then it will be
+        unquoted. Otherwise it's returned as-is.
+        """
+        result = self.QUOTE_REGEX.match(item)
+        if result:
+            return result.group('value')
+        return item
 
     def serialize(self, value):
         """Serialize ``value`` into a multi-line string."""
         if not isinstance(value, (list, set)):
             raise ValueError('ListAttribute value must be a list.')
+        elif not value:
+            # return an empty string when there is no value
+            return ''
 
         # we ensure to read a newline, even with only one value in the list
         # this way, comma will be ignored when the configuration file
         # is read again later
-        return '\n' + '\n'.join(value)
+        return '\n' + '\n'.join(self.serialize_item(item) for item in value)
+
+    def serialize_item(self, item):
+        """Serialize an ``item`` from the list value.
+
+        :param str item: one item of the list to serialize
+        :rtype: str
+
+        If ``item`` starts with a ``#`` it will be quoted in order to prevent
+        the config parser from thinking it's a comment.
+        """
+        if item.startswith('#'):
+            # we need to protect item that would otherwise appear as comment
+            return '"%s"' % item
+        return item
 
     def configure(self, prompt, default, parent, section_name):
         each_prompt = '?'

test/test_config.py

@@ -2,6 +2,7 @@
 from __future__ import unicode_literals, division, print_function, absolute_import
 
 import os
+import sys
 
 import pytest
 
@@ -26,8 +27,43 @@
     cheddar
       reblochon   
   camembert
+channels =
+    "#sopel"
+    &peculiar
+# regular comment
+    # python 3 only comment
+    "#private"
+    "#startquote
+    &endquote"
+    "&quoted"
 """  # noqa (trailing whitespaces are intended)
 
+TEST_CHANNELS = [
+    '#sopel',
+    '&peculiar',
+    '#private',
+    '"#startquote',  # start quote withtout end quote: kept
+    '&endquote"',
+    '"&quoted"',  # quoted, but no #: quotes kept
+]
+
+if sys.version_info.major < 3:
+    # Python 2.7's ConfigParser interpret as comments
+    # a line that starts with # or ;.
+    # Python 3 on the other hand, allow comments to be indented.
+    # As a result, the same config file will result in a different
+    # config file depending on the Python version used.
+    # TODO: Deprecated with Python 2.7.
+    TEST_CHANNELS = [
+        '#sopel',
+        '&peculiar',
+        '# python 3 only comment',  # indented line are not comment for Py2
+        '#private',
+        '"#startquote',
+        '&endquote"',
+        '"&quoted"',
+    ]
+
 
 class FakeConfigSection(types.StaticSection):
     valattr = types.ValidatedAttribute('valattr')
@@ -43,6 +79,7 @@ class SpamSection(types.StaticSection):
     eggs = types.ListAttribute('eggs')
     bacons = types.ListAttribute('bacons', strip=False)
     cheeses = types.ListAttribute('cheeses')
+    channels = types.ListAttribute('channels')
 
 
 @pytest.fixture
@@ -219,6 +256,8 @@ def test_configparser_multi_lines(multi_fakeconfig):
         'camembert',
     ]
 
+    assert multi_fakeconfig.spam.channels == TEST_CHANNELS
+
 
 def test_save_unmodified_config(multi_fakeconfig):
     """Assert type attributes are kept as they should be"""
@@ -257,6 +296,7 @@ def test_save_unmodified_config(multi_fakeconfig):
         'reblochon',
         'camembert',
     ]
+    assert saved_config.spam.channels == TEST_CHANNELS
 
 
 def test_save_modified_config(multi_fakeconfig):
@@ -269,6 +309,14 @@ def test_save_modified_config(multi_fakeconfig):
     multi_fakeconfig.spam.cheeses = [
         'camembert, reblochon, and cheddar',
     ]
+    multi_fakeconfig.spam.channels = [
+        '#sopel',
+        '#private',
+        '&peculiar',
+        '"#startquote',
+        '&endquote"',
+        '"&quoted"',
+    ]
 
     multi_fakeconfig.save()
 
@@ -287,3 +335,11 @@ def test_save_modified_config(multi_fakeconfig):
         'ListAttribute with one line only, with commas, must *not* be split '
         'differently from what was expected, i.e. into one (and only one) value'
     )
+    assert saved_config.spam.channels == [
+        '#sopel',
+        '#private',
+        '&peculiar',
+        '"#startquote',  # start quote withtout end quote: kept
+        '&endquote"',
+        '"&quoted"',  # doesn't start with a # so it isn't escaped
+    ]