PhileCMS / Phile

A flat file CMS with a swappable parser and template engine.
https://philecms.github.io/
Other
257 stars 49 forks source link

load settings if not loaded #113

Closed STOWouters closed 10 years ago

STOWouters commented 10 years ago

Not sure if it's a bug or a feature, but the sorting didn't work because $this->settings was always empty, so I forced it to load the settings (if it's not already loaded) so the $config['pages_order'] can be retrieved.

Schlaefer commented 10 years ago

The $settings init in \Repository\Page not 100% perfect, I thought I had it covered in the __constructor. But there was another change in that regard already.

At the moment I'm not aware in which case it will not get initialized. It was my impression that it's not possible for $config['pages_order'] to be empty, because a default value is set in default_config.php.

STOWouters commented 10 years ago

Yeah, I discovered the \Repository\Page constructor were called twice: on the first call it has actually got the settings loaded, but on the second call, the $this->settings became NULL.

james2doyle commented 10 years ago

@Stijn-Flipper can you patch this?

STOWouters commented 10 years ago

I've reverted it back and patched the constructor instead.

STOWouters commented 10 years ago

Hmm, is it safe to revert this commit ? Because the settings will never be NULL anyway.

Schlaefer commented 10 years ago

I think I see the problem: in the __constructer I forgot to set $settings if it is passed, which is the case when called from the twig template plugin. Change it to:

if ($settings === null) {
            $settings = \Phile\Registry::get('Phile_Settings');
}
$this->settings = $settings;

Stupid bug by me.

NeoBlack commented 10 years ago

@Stijn-Flipper can you use the code fragment of @Schlaefer and push it to your branch? I think, then it is ok and we can merge it.

STOWouters commented 10 years ago

@NeoBlack It's already fixed.

NeoBlack commented 10 years ago

ok, then we can close this issue?

STOWouters commented 10 years ago

You have to merge this yet :)

NeoBlack commented 10 years ago

+1 ok, now I understand :D

james2doyle commented 10 years ago

Just tested this after merging #110 and got the following errors.

[8] Undefined offset: 1 in /Users/james2doyle/Sites/PhileCMS/lib/Phile/Repository/Page.php on line 112 [8]
[2] constant(): Couldn't find constant SORT_ in /Users/james2doyle/Sites/PhileCMS/lib/Phile/Repository/Page.php on line 132 [2]
[2] array_multisort(): Argument #4 is expected to be an array or a sort flag in /Users/james2doyle/Sites/PhileCMS/lib/Phile/Repository/Page.php on line 136 [2]
[8] Undefined offset: 1 in /Users/james2doyle/Sites/PhileCMS/lib/Phile/Repository/Page.php on line 112 [8]
[2] constant(): Couldn't find constant SORT_ in /Users/james2doyle/Sites/PhileCMS/lib/Phile/Repository/Page.php on line 132 [2]
[2] array_multisort(): Argument #4 is expected to be an array or a sort flag in /Users/james2doyle/Sites/PhileCMS/lib/Phile/Repository/Page.php on line 136 [2]
[8] Undefined offset: 1 in /Users/james2doyle/Sites/PhileCMS/lib/Phile/Repository/Page.php on line 112 [8]
[2] constant(): Couldn't find constant SORT_ in /Users/james2doyle/Sites/PhileCMS/lib/Phile/Repository/Page.php on line 132 [2]
[2] array_multisort(): Argument #4 is expected to be an array or a sort flag in /Users/james2doyle/Sites/PhileCMS/lib/Phile/Repository/Page.php on line 136 [2]

This was after a new install and I used the code from the pagination how-to wiki in my index.html template.

NeoBlack commented 10 years ago

I will close this pull request, please create a new one with target master branch. today we change the developer workflow. please look into our wiki for more informations. thank your for your help! please create a reference to this pull request from your new pull request.