Issue 6552 - Support arbitrary manifest values

chainedconfigparser.py

 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import os
 import io
 import ConfigParser
 from StringIO import StringIO
-from collections import OrderedDict
 
 
 class Item(tuple):
     def __new__(cls, name, value, source):
         result = super(Item, cls).__new__(cls, (name, value))
         result.source = source
         return result
 
 
 class DiffForUnknownOptionError(ConfigParser.Error):
@@ skipping 67 matching lines @@
         is_addition = option.endswith('+')
         is_diff = is_addition or option.endswith('-')
 
         if is_diff:
             option = option[:-1].rstrip()
             try:
                 orig_value = self.get(section, option)
             except ConfigParser.NoOptionError:
                 raise DiffForUnknownOptionError(option, section)
 
-            orig_values = orig_value.split()
-            diff_values = value.split()
+            orig_values = orig_value.splitlines()
+            diff_values = value.splitlines()
 
             if is_addition:
                 new_values = orig_values + [v for v in diff_values if v not in orig_values]
             else:
                 new_values = [v for v in orig_values if v not in diff_values]
 
-            value = ' '.join(new_values)
+            value = '\n'.join(new_values)
 
         return is_diff, option, value
 
     def _process_parsers(self, parsers):
         for parser, filename in parsers:
             for section in parser.sections():
                 if not self.has_section(section):
                     try:
                         ConfigParser.SafeConfigParser.add_section(self, section)
                     except ValueError:
@@ skipping 35 matching lines @@
 
     def option_source(self, section, option):
         option = self.optionxform(option)
         try:
             return self._origin[(section, option)]
         except KeyError:
             if not self.has_section(section):
                 raise ConfigParser.NoSectionError(section)
             raise ConfigParser.NoOptionError(option, section)
 
-    def as_json_object(self, section):

[Sebastian Noack, 2018/04/14 00:33:07]

Actually there isn't anything specific to JSON in this function. It just returns
a dictionary which then is converted to JSON in the template.

[tlucas, 2018/04/14 08:36:23]

You are right - what would you suggest? How about merge_to_nested()?

[Sebastian Noack, 2018/04/14 08:54:26]

Maybe section_as_dict()?

[tlucas, 2018/04/18 14:46:04]

Done.

-        r"""Parse a given section into a JSON object.
+    def serialize_section_if_present(self, section, base):

[Vasily Kuznetsov, 2018/04/19 14:40:10]

What this method does seems more like parsing than serialization. Perhaps it
should be called "parse_section_if_present".

[Sebastian Noack, 2018/04/19 15:02:59]

In the end it gets serialized (as JSON). But you might be right that what we do
here is parsing it from the config. I guess, its too late now. Though, if
anybody feeling strong about this and submits a Noissue patch, I would take it.

+        """Serialize a given section as a dictionary into `base`.
 
         Parse arbitrary key/value pairs from 'section' of the current
-        configuration into a nested JSON object.
+        configuration into a dictionary and deep merge it into `base`.
 
         The following rules need to be considered:
 
         * An option's key may be declared as a series of nested dictionary keys,
           seperated by '.'.
-        * An option's key must end with '[]', to mark this option as an array

[Sebastian Noack, 2018/04/14 00:33:06]

Nit: Please make all bullet point end with punctation, for consistence.

[Sebastian Noack, 2018/04/14 00:33:07]

I just noticed, we could identify arrays by the presence of newlines, rather
than by adding [] to the key:

  foo = just a string

  foo =
    1st item of array
    2nd item of array

  foo =
    array with only one item

What do you think?

[tlucas, 2018/04/14 08:36:23]

The bullet points: ack

The newlines: Nice idea, will do

[tlucas, 2018/04/18 14:46:04]

Done.

-        * When an option is marked as an array, no other nested objects may
-          follow
-        * An array is expandable by the ConfigParser's '+=' token.
+        * Declaring an option's value in a new line (even if only one is given)
+          will define the option's value as a list.
+        * When an option's value is defined as a list, no other nested
+          objects may follow.
+        * A list is expandable by the ConfigParser's '+=' token (Note: A
+          previously declared string will be converted into a list).
         * Values may be marked as `number` or `bool` by prefixing them
-          accordingly:
+          accordingly (this also applies to values in a list):
           * bool:<value>
           * number:<value>
 
         Example:
-
-                                        {
-                                          "good": false,
-                                          "bar": {
-                                            "baz": ["bar", "bazinga"]
-        foo = foo                         },
-        bar.baz[] = bar                   "is": {
-        baz.foo = a                         "integer": 1,
-        baz.z[] = b bool:true               "float": 1.4
-        bar.baz[] += bazinga    ==>       },
-        bad = bool:true                   "baz": {
-        good = bool:false                   "foo": "a",
-        is.integer = number:1               "z": ["b", true]
-        is.float = number:1.4             },
-                                          "bad": true
-                                          "foo": "foo"
-                                        }
+                                    {
+        foo = foo                     "foo": "foo",
+        asd =                         "asd": ["asd"],
+          asd                         "bar": {
+        bar.baz = a                     "baz": ["a", "c", "d"]
+        baz.foo = a                   },
+        baz.z =                       "baz": {
+          bar                           "foo": "a",
+          bool:true             ===>    "z": ["bar", true]
+        bar.baz +=                    },
+          c                           "bad": true,
+          d                           "good": false,
+        bad = bool:true               "is": {
+        good = bool:false               "integer": 1,
+        is.integer = number:1           "float": 1.4
+        is.float = number:1.4         }
+                                    }
         """
-        def parse_values(v):
-            if isinstance(v, list):
-                return [parse_values(x) for x in v]
-
+        def parse_value(v):
             if v.startswith('number:'):
                 v = v.split(':', 1)[1]
                 try:
-                    v = int(v)
+                    return int(v)
                 except ValueError:
-                    v = float(v)
-            elif v == 'bool:true':
-                v = True
-            elif v == 'bool:false':
-                v = False
+                    return float(v)
+            if v == 'bool:true':
+                return True
+            if v == 'bool:false':
+                return False
             return v
 
-        def setdefault_recursive(target, arr):
-            if len(arr) == 2:
-                target.setdefault(arr[0], parse_values(arr[1]))
-            else:
-                current = target.setdefault(arr[0], OrderedDict())
-                setdefault_recursive(current, arr[1:])
-
-        data = self.items(section)
-        result = OrderedDict()
-
-        for k, v in data:
-            if k.endswith('[]'):
-                k = k[:-2]
-                v = v.split()

[Sebastian Noack, 2018/04/14 00:33:06]

We might want to strip by newlines, so that array items can have spaces.

[tlucas, 2018/04/14 08:36:23]

Acknowledged.

[tlucas, 2018/04/18 14:46:04]

Done.

-
-            setdefault_recursive(result, k.split('.') + [v])

[Sebastian Noack, 2018/04/14 00:33:06]

It seems weird/unnecessary to merge the key segments and value into one list.
Any reason to not pass them as separate arguments?

[tlucas, 2018/04/14 08:36:23]

Not at all, will change.

[tlucas, 2018/04/18 14:46:04]

Done.

-
-        return result
+        if self.has_section(section):
+            for k, v in self.items(section):
+                parents = k.split('.')
+                tail = parents.pop()
+                current = base
+                for name in parents:
+                    current = base.setdefault(name, {})
+
+                if '\n' in v:
+                    current[tail] = [parse_value(x) for x in v.splitlines() if x]
+                else:
+                    current[tail] = parse_value(v)
 
     def readfp(self, fp, filename=None):
         raise NotImplementedError
 
     def set(self, section, option, value=None):
         raise NotImplementedError
 
     def add_section(self, section):
         raise NotImplementedError
 
     def remove_option(self, section, option):
         raise NotImplementedError
 
     def remove_section(self, section):
         raise NotImplementedError