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

Blueprint - Support logic with PHP #979

Closed
jenstornell opened this issue Dec 23, 2016 · 7 comments
Closed

Blueprint - Support logic with PHP #979

jenstornell opened this issue Dec 23, 2016 · 7 comments
Labels

Comments

@jenstornell
Copy link

https://forum.getkirby.com/t/kirby-blueprint-logic/6127/9

Possible solution

Blueprint file

project.php - a blueprint file.

Blueprint content

return array(
);

So it basically just returns an array from the php-version of the blueprint.

Benefits:

  • Because it's just an array it should be secure.
  • Because it's an array, there is no need to yaml decode it as it's already an array.
  • Because it's returned, it's possible like a controller to populate it by logic (loops etc).

@lukasbestle:
What you proposed for the core is exactly what I thought as well, we would just need to make it backwards-compatible with existing YAML blueprints with the .php extension.

@lord-executor
Copy link

Just did a quick test on that topic. All I changed in the panel code was models/page/blueprint.php:69 which I turned into this:

$this->file = $file;
$this->name = $name;
if (pathinfo($file, PATHINFO_EXTENSION) === 'php') {
    $this->yaml = require($this->file);
} else {
    $this->yaml = data::read($this->file, 'yaml');
    // remove the broken first line
    unset($this->yaml[0]);
}

With that I then added a test blueprint test.php with code like this:

<?php

$raw = data::read(__DIR__ . DS . 'error.yml', 'yaml');
unset($raw['fields']['intro']);

return $raw;

And this already works perfectly.

@lukasbestle
Copy link
Member

lukasbestle commented Dec 24, 2016

@lord-executor No, unfortunately that won't work as old YAML blueprints can also have the .php extension. We introduced .yml later.

So what we'd need to do to keep it backwards-compatible is to use output-buffering when including the PHP file and then check if there is a return value. If not, we'd need to decode the YAML from the output buffer.

I'm not sure though if that's a good idea. Maybe we should just kill backwards-compatibility here.

@lord-executor
Copy link

Ah yes... I remember seeing .php blueprints in older Kirby installations. How about making the extension for acutal PHP logic blueprints to ".yml.php" and checking for that?

Or... as you suggested: kill backwards compatibility. For an upgrade path it would certainly be possible to write a bash one-liner that changes the extension of all "old school" .php blueprints to .yml .

@jenstornell
Copy link
Author

@lukasbestle

Maybe we should just kill backwards-compatibility here.

I totally agree. The alternative sounds a little messy for no real gain. Using the php extension for yml feels like a relic from the past.

Is it easy to check if the returned array is valid? In that case maybe add a message in the panel if it fails.

Example:

blueprint projects.php failed to load!
Since Kirby 2.5 this file should return a valid array.
If it contains yml data, simply rename it from projects.php to projects.yml

That could save a few questions in the forum.

@lukasbestle
Copy link
Member

lukasbestle commented Dec 26, 2016

I agree that it's better to remove that old behavior to keep this clean.

Unfortunately we can't check if the blueprint is valid. To validate the returned array, we need to require the file. YAML blueprints will then dump their content to the screen. Only way to get around this is to use, well, output buffering...

@jenstornell
Copy link
Author

jenstornell commented Dec 27, 2016

Ahh, now I see why that is. Then I guess we just need @bastianallgeier to agree that this breaking change is a right way forward.

Page object

Another thing releated to this, maybe we should have access to the $page object similar to a controller? Then we could use data from the current page content or the parent page content when generating the blueprint.

Maybe it's not always good to depend the current blueprint on the current page content, but using data from the parent could be useful, or something like echo page('data-' . $page->slug())->some_value(); in the blueprint.

Anyway, there are benefits that comes with access of the page object, but if there are concerns that people will depend too much on itself, maybe add a warning about it in the docs. I mean, changing a value that is changing the blueprint that is then corrupting the blueprint. Theoretically a break like that would be possible. A great blueprint "architect" should be aware of that risk.

@lord-executor
Copy link

If you are looking at blueprints as just "determining the displayed fields in the panel UI", then this could work in some situations. But it could also lead to some (un-) funny bootstrapping / dependency issues if you take it too far. Then again, I've always been following the "it's your foot" philosophy ;)

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

No branches or pull requests

4 participants