Skip to content

allow ini files to have separators between sections for better readability #905

Merged
merged 1 commit into from Oct 23, 2012

3 participants

@dereuromark
CakePHP member

currently, the whole file is one list of configurations without the natural space between sections you usually have with ini files in unix.
those make the files easier to read and edit by humans.

so I tried to add separator. it might not have been the best approach so far.
If you got any better ideas, shoot.
I guess, grouping them into "section" arrays before imploding would probably make the code better and less likely to error.

@ADmad
CakePHP member
ADmad commented Oct 16, 2012

You will have to modify Configure::dump() also to be able to pass the extra options. Config readers aren't used directly generally.

@ADmad ADmad commented on an outdated diff Oct 16, 2012
lib/Cake/Configure/IniReader.php
* @return int Bytes saved.
*/
- public function dump($filename, $data) {
+ public function dump($filename, $data, $options = array()) {
+ if (isset($options['separator']) && $options['separator'] === false) {
+ unset($options['separator']);
+ } elseif (isset($options['separator']) && is_bool($options['separator'])) {
+ $options['separator'] = '';
+ }
@ADmad
CakePHP member
ADmad added a note Oct 16, 2012

Instead of all these checks just make defaults array and merge passed options with defaults.

$defaults = array('separator' => null);
$options += $defaults;
@ADmad
CakePHP member
ADmad added a note Oct 16, 2012

Also not sure if this boolean true to empty string conversion is necessary. Can just directly specify separator like array('separator' => '').
You can use either null or false to denote no separator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dereuromark
CakePHP member

@ADmad Well, my classes use them directly :) Since they need to write and parse ini files.
The main goal was to allow them to use the core class with this enhancement.
But I agree, that Configure::dump() then should also profit from the add-on.

@markstory
CakePHP member

Do we really need an option here? I don't see why this couldn't & shouldn't just be the default. It does not change the end results one bit. And customizing the separator seems like an ignorable edge case.

@dereuromark
CakePHP member

yeah, but then I should really rewrite it in a clean way. the unsetting part is really ugly.

@markstory
CakePHP member

I would remove it. I don't see why you couldn't just append a blank row after each section. Then once the ini file is imploded run trim() on it. Seems like that gets rid of a bunch of conditional logic and leaves a file with no trailing whitespace.

@dereuromark
CakePHP member

something like that, right?

@markstory
CakePHP member

Yep, just like that 👍

@dereuromark dereuromark allow ini files to have separators between sections for better readab…
…ility

dump now makes spaces between sections in init files
5a8092f
@dereuromark
CakePHP member

I squashed it pretty quick.

@markstory markstory merged commit d75bd48 into cakephp:2.3 Oct 23, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.