search

nikoutel / HelionConfig

A versatile configuration parser. Can handle multiple configuration formats.
Mozilla Public License 2.0
5 stars 2 forks source link

Config values overwrite when they have the same key #1

Open rickschippers opened 5 years ago

rickschippers commented 5 years ago

I'm using the library to parse apache configurations. I have a configuration like this:

<VirtualHost *:80>
    ServerAdmin foo@bar.com

    DocumentRoot /var/www/vhosts/default/web

    ErrorLog ${APACHE_LOG_DIR}/error.log
    CustomLog ${APACHE_LOG_DIR}/access.log combined

    Include conf-available/php7.1-fpm.conf
    Include expires.conf
</VirtualHost>

When this is parsed the VirtualHost.Include config will have the expires.conf. During parsing it has overwritten the conf-available/php7.1-fpm.conf, since the are both stored in in the Include key. But since you can have multiple Include statements in apache configuration they should not overwrite.

nikoutel commented 5 years ago

Thanks for catching that.

I've pushed a fix to branch dev. Have a look at it. When I have more time I'll do some more testing and merge to master.

rickschippers commented 5 years ago

I tested dev-dev@dev. That doesn't fix the issue yet. I made a pull request with a possible solution:

https://github.com/nikoutel/HelionConfig/pull/2

rickschippers commented 5 years ago

Seem like dev-dev@dev through composer still gives me an older commit. Let me check again with your fix.

nikoutel commented 5 years ago

I didn't update composer. Now I have. Can you test again please.

rickschippers commented 5 years ago

dev-dev@dev now gives me your commit with the fix. That indeed fixes the issue. It will parse correct.

[VirtualHost.Include.0] => conf-available/php7.2-fpm.conf
[VirtualHost.Include.1] => expires.conf
nikoutel commented 5 years ago

Great! I guess its time for those GitHub Hooks for Packagist. I have always postponed this.