sauliusgrigaitis / Swifton

A Ruby on Rails inspired Web Framework for Swift that runs on Linux and OS X
MIT License
1.97k stars 69 forks source link

Add support for Stencil include tag #26

Closed califrench closed 8 years ago

califrench commented 8 years ago

By giving all Stencil views a default context with a TemplateLoader we can add global support for the Stencil {% include ... %} tag. This saves us from having to create a template loader on each request or having to modify the ApplicationController like so:

import Swifton
import Inquiline
import Stencil
import PathKit

class ApplicationController: Controller {
    override init() { super.init() }

    override func render(template: String, _ context: [String: Any]) -> Response {
        var newContext = context
        newContext["loader"] = TemplateLoader(paths: ["Views"])
        return super.render(template, newContext)
    }

    override func render(template: String) -> Response {
        return super.render(template, ["loader": TemplateLoader(paths: ["Views"]), "page": Path(template).components.first?.lowercaseString])
    }

}

Please refer to Stencil for more information.

sauliusgrigaitis commented 8 years ago

@califrench thanks! Could you also extend this PR with meaningful use case covered with Controller spec?

sauliusgrigaitis commented 8 years ago

And README update for sure :)

califrench commented 8 years ago

This sucks because I can't build on OS X. I get

Swifton/Packages/Nimble-3.1.1/Sources/Nimble/Adapters/NimbleXCTestHandler.swift:2:8: error: cannot load underlying module for 'XCTest'
import XCTest

So I have no way to run the tests locally...

sauliusgrigaitis commented 8 years ago

Yes, it's not the best situation really. https://github.com/Quick/Quick/issues/458 doesn't show any progress. So probably best would be to run Docker or VM with mounted volume, so you can run tests in docker, and proceed with your changes in OSX.

Current idea of test suite is to have small set of "integrated" tests that shows meaningful usage. So if you could wrap existing Index action with header/footer then it would be much more meaningful than separate Incude action. Please let me know if you can't get tests running in Docker or VM - I'll write it myself.

califrench commented 8 years ago

Hi Saulius,

I tried to get it to work several times with the default Index action but since I couldn't test locally, I had to keep committing and pushing to the repo so the tests could run with Travis CI. This is obviously terrible in practice so I decided to minimize the test case to test the inclusion very specifically... Originally I wrote the test as the following: ControllerSpec.swift

it("Renders HTML from Stencil template with an include") {
    let rendered = controller["include"](request: request)
    expect(rendered.body).to(equal("<h1>Included Template Title</h1>\n\nSaulius\n\nJames\n\n\n"))
}

And in TestModelsController

action("include") { request in
    let testModels = ["testModels": TestModel.allAttributes()]
    return render("TestModels/Include", testModels) 
}

Now regarding wrapping the existing Index action, I can't really do that since the Include.html.stencil template has an {% include "Head.html.template" %} tag. So I would need to modify the Index.html.stencil template to add that tag, which would cause all the other tests to fail.

In short, the way my original test was designed was much more meaningful but given the fact I spent several hours trying to get the test working correctly and couldn't do that effectively without being able to run the tests locally, I made my test case much simpler.

It would be great if you could revert this back to commit 0e837ec to provide a meaningful test scenario. After you revert to that commit, it's just a matter of figuring out how many "\n" Stencil renders when including another template.

sauliusgrigaitis commented 8 years ago

@califrench thanks for your efforts, please just delete your commits until 0887392 and I'll merge it.

Until Quick is fixed I really suggest to use VM or Docker.

califrench commented 8 years ago

@sauliusgrigaitis Done! I wish I could have been more helpful for this issue. I'll look into Docker!

sauliusgrigaitis commented 8 years ago

@califrench thanks!