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

Trimming output? #44

Closed
barryo opened this issue Oct 31, 2016 · 5 comments
Closed

Trimming output? #44

barryo opened this issue Oct 31, 2016 · 5 comments
Assignees

Comments

@barryo
Copy link
Contributor

barryo commented Oct 31, 2016

Hi!

Thanks for creating Foil. It solves a big issue I had with Plates: lack of multi-folder support and so creating skinnable templates with Plates was not possible.

One thing with Foil that keeps biting me is this line which is:

    private function doRender($path, array $data = [], $class = null)
    {
        // ...
        $output = trim($template->render($data));
        // ...
        return $output;
    }

Why trim()? Can this not be left up to the developer as desired? E.g. me! Or made an option? One of things we use foil for (as well as HTML templates) is building plain text configuration files and whitespace is important here.

Thanks, Barry.

@gmazzap gmazzap self-assigned this Oct 31, 2016
@gmazzap
Copy link
Contributor

gmazzap commented Oct 31, 2016

Hi @barryo the trim was added to make comparing templates easier in unit tests, but I see more people using Foil as code builder, which was not what I had in mind, but fine :), so it makes sense.

Spaces are not a big issue in PHP so I can remove that trim without backward compatibility issue.

Don't really have time right know, happy to merge a PR to dev if you have time to send it.

barryo added a commit to barryo/Foil that referenced this issue Oct 31, 2016
gmazzap added a commit that referenced this issue Nov 1, 2016
Do not trim output - fixes #44
@gmazzap
Copy link
Contributor

gmazzap commented Nov 1, 2016

Done with 1169c54, thanks @barryo

@gmazzap gmazzap closed this as completed Nov 1, 2016
gmazzap added a commit that referenced this issue Nov 1, 2016
Completing trim() fix in #44 which was only partly solved by #45
@barryo
Copy link
Contributor Author

barryo commented May 3, 2017

Hey @gmazzap - any plans to merge this into master / release?

We're shipping IXP Manager with a foil-dev requirement for a while now.

Thanks a million,
Barry

@gmazzap
Copy link
Contributor

gmazzap commented May 3, 2017

I will merge very soon.

@barryo
Copy link
Contributor Author

barryo commented May 4, 2017

Thanks for that @gmazzap !

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

No branches or pull requests

2 participants