New Piwik Ini component #6939
New Piwik Ini component #6939
Conversation
mnapoli
added
the
Enhancement
|
Great idea, this change makes our code less cluttered and cleaner, and also will benefit the community, it is a great move 👍 (fyi didn't review the code in detail, but I love that we will have tests now!) |
|
@mnapoli you say "works even if parse_ini_file() is disabled" - maybe we need to add some checks if function exists instead of writing own function, because native function much faster. And $array = @parse_ini_string($ini, true, INI_SCANNER_RAW); what is it? Suppress an error output is bad idea I'm think. |
|
@Globulopolis thanks for the feedback. It does use the native PHP function, the shim is just used if the function is disabled. Regarding $array = @parse_ini_string($ini, true, INI_SCANNER_RAW);
if ($array === false) {
$e = error_get_last();
throw new IniReadingException('Syntax error in INI configuration: ' . $e['message']);
}I'm actually checking for the error just afterwards and turning it into an exception. So the error is not silenced, it's turned into an exception. |
mnapoli
added
the
Pull Request WIP
|
The new component is here: https://github.com/piwik/component-ini |
mnapoli
removed
the
Pull Request WIP
|
Nice, gotta love new small reusable components keeping the core lean 👍 |
This PR is a suggestion for a new Piwik component to read and write INI configurations.
The new component contains 2 classes:
IniReader: content extracted from the shim inupgrade.phpthat makes it work even if the native PHP function is disabledIniWriter: content extracted fromConfig(so it clears up this class a little bit)Just like Symfony Yaml, I think this INI component could be interesting to be shipped as a standalone component. It's a good example of a small reusable one.
Here is the advantages of this component that I would write in its README:
parse_ini_file()is disabledif ( === false)…)true,yes,onare actually converted totrueinstead of1, same forfalse,no,offHere is the advantage for us:
ConfigThoughts?