Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken prepare #57

Closed
ilyachase opened this issue Oct 25, 2018 · 2 comments
Closed

Broken prepare #57

ilyachase opened this issue Oct 25, 2018 · 2 comments

Comments

@ilyachase
Copy link

ilyachase commented Oct 25, 2018

Hi!
In "3.14" path you broke simple case of preparing templates.
I'm talking about these changes: http://i.imgur.com/wm6WnFP.png

Let's say we have simple class:

class ViewParser extends \portalbladeone\Base
{
    public function __construct($templatesPath, $templatesCompiledPath)
    {
        $this->file = str_replace('.blade.php', '', $file);
        parent::__construct($templatesPath, $templatesCompiledPath);
    }
}

And if you try to do (new ViewParser('/path/to/templates/','/path/to/compiled/templates'))->compile('my_awesome_template', true);, you will get error:
portalbladeone\Exception\BladeError: Read file : Template not found :
Since you deleted $this->fileName = $fileName; part, $compiled = $this->getCompiledFile(); and $template = $this->getTemplateFile(); will get trash.

Also doesn't it do more sense to switch condition from if ($this->isExpired() || $forced) to if ($forced || $this->isExpired())

I can make test and pull request to fix this.

@ilyachase
Copy link
Author

Also we can add $fileName = null parameter to public function isExpired() function and pass filename from compile to it.
But still we don't event need to check expiration if we 'forced' to compile.

jorgecc pushed a commit that referenced this issue Oct 25, 2018
@jorgecc
Copy link
Member

jorgecc commented Oct 25, 2018

  • fixed
  • if ($this->isExpired() || $forced) to if ($forced || $this->isExpired())
  • unit test

@jorgecc jorgecc closed this as completed Oct 25, 2018
jorgecc pushed a commit that referenced this issue Oct 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants