Skip to content
This repository has been archived by the owner on Jul 25, 2022. It is now read-only.

Semigroup could combine sections #31

Open
chris-martin opened this issue Aug 1, 2021 · 1 comment
Open

Semigroup could combine sections #31

chris-martin opened this issue Aug 1, 2021 · 1 comment

Comments

@chris-martin
Copy link
Contributor

chris-martin commented Aug 1, 2021

When semigroupally combining Ini values that both define the same section, I was surprised to find that one of the sections is discarded. The monoid instance currently looks like this:

mappend x y = Ini {iniGlobals = mempty, iniSections = iniSections x <> iniSections y}

I was expecting sections of the same name to be merged.

Also, why discard all the globals?

Proposed change to the monoid instance:

mappend x y = Ini {
    iniGlobals = Map.unionWith (<>) (iniGlobals x) (iniGlobals y),
    iniSections = Map.unionWith (<>) (iniSections x) (iniSections y) }

Either way, whether you are interested in changing the instance or not, I'd be happy to send a PR with some documentation to mention how the instance behaves.

@chrisdone
Copy link
Collaborator

Good catch, classic map appending bug. Please PR and I’ll merge!

👌🙂

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants