ImageConfigParser: Don't sort merged values

[dbnicholson]

The order of configuration values may be significant, so the parser should not sort them. To do that, use a single counter for add and del variants and emit the values that have positive counts without sorting. This relies on the Python dict stable order, which has been the case since Python 3.6.

I've done no real world testing of this. It should also be stated that the order you get is "first merged section wins", so it's not really reliable. For example:

[section]
# From product config
option_add_product = foo
# From local product config
option_add_local_product = bar

You'd get foo\nbar even though you might really prefer bar\nfoo from your local setting. With explicit sorting you'd always get the same values regardless of what order the unmerged options came in.

[wjt]

I'll look more closely at this on Monday.

At a first glance it looks very neat, and it solves part of the problem discussed on #169 – without this change, if we make the Kolibri channels field mergeable then it's impossible to preserve order.

The harder part is that in the derived-personality case, you might want (e.g.) en_GB.ini to add 2 more channels to the set provided by en.ini, but you may not want them to go after all the channels from en.ini. I think the way you would deal with that is to first _del all the inherited channels (removing them from the counter), then explicitly add them all back along with the 2 extra ones. Or explicitly set the field (which I think overrides the merging? or could be made to). Not ideal but it's not (substantially) worse than what we have now.

[dbnicholson]

The harder part is that in the derived-personality case, you might want (e.g.) en_GB.ini to add 2 more channels to the set provided by en.ini, but you may not want them to go after all the channels from en.ini. I think the way you would deal with that is to first _del all the inherited channels (removing them from the counter), then explicitly add them all back along with the 2 extra ones. Or explicitly set the field (which I think overrides the merging? or could be made to). Not ideal but it's not (substantially) worse than what we have now.

Indeed, it's currently not possible to specify the desired semantics of the configuration, but it's also pretty hard to implement them. You can always fully override at any time by using the unsuffixed version, but that's probably not what you want in most cases. Fortunately, this is a part of the code that has tests, so you can throw whatever crazy stuff in there and check the behavior.

[wjt]

I rebased this backwards so that it can be merged onto eos6.0.

[wjt]

I was curious whether Counter provides a shorter way to spell “only keys that have positive values” and it does:

In [7]: values = Counter(foo=3, bar=-1)

In [8]: +values
Out[8]: Counter({'foo': 3})

In [9]: '\n'.join(+values)
Out[9]: 'foo'

So we could write:

Suggested change

	sect[option] = '\n'.join(k for k, v in values.items() if v > 0)
	sect[option] = '\n'.join(+values)

But I've never seen this before and I think it's too obscure.