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.

Sign up for GitHub

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

Avoid Helpers::Render in mandatory includes #610

Closed
wants to merge 9 commits into from
Closed

Avoid Helpers::Render in mandatory includes #610

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d336d44
Avoid Helpers::Render in mandatory includes
docelic Feb 1, 2018
547809c
Correct module name
docelic Feb 1, 2018
714755e
Merge branch 'master' into move-render-helper
faustinoaq Feb 3, 2018
6b1987e
Add missing requires/includes
docelic Feb 3, 2018
805badf
Merge branch 'move-render-helper' of github.com:docelic/amber into mo…
docelic Feb 3, 2018
a635d60
Fix require path
docelic Feb 3, 2018
35777fb
Merge branch 'master' into move-render-helper
faustinoaq Feb 3, 2018
4ebfa8e
Fix specs
Feb 3, 2018
85052e0
Merge branch 'master' into move-render-helper
faustinoaq Feb 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions spec/support/fixtures/controller_fixtures.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class HelloController < Amber::Controller::Base
end

class RenderController < Amber::Controller::Base
include Amber::Controller::Helpers::Render
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add an ApplicationController fixture in between?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good one, will do, thanks!

Copy link
Contributor Author

@docelic docelic Feb 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually @drujensen looking into spec/support/fixtures/controller_fixtures.cr I suggest things to remain as they are.

Because if we add ApplicationController fixture then other fixture classes in that file should inherit from it, which would make them have more functionality than the spec specifically wanted to test.

So I would for keeping specs as granular and specific as they are now.

def render_template_page
render("spec/support/sample/views/test/test.slang", layout: false)
end
Expand Down
2 changes: 0 additions & 2 deletions src/amber.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ require "logger"
require "json"
require "colorize"
require "random/secure"
require "kilt"
require "kilt/slang"
require "redis"
require "./amber/version"
require "./amber/controller/**"
Expand Down
2 changes: 2 additions & 0 deletions src/amber/cli/templates/app/src/controllers/application_controller.cr.ecr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
require "amber/controller/helpers/render"
require "jasper_helpers"

class ApplicationController < Amber::Controller::Base
include Amber::Controller::Helpers::Render
include JasperHelpers
LAYOUT = "application.<%= @language %>"
end
6 changes: 4 additions & 2 deletions src/amber/controller/base.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
require "http"

require "./filters"
require "./helpers/*"
require "./helpers/csrf"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this. ❤️

require "./helpers/redirect"
require "./helpers/responders"
require "./helpers/route"

module Amber::Controller
class Base
include Helpers::CSRF
include Helpers::Redirect
include Helpers::Render
include Helpers::Responders
include Helpers::Route
include Callbacks
Expand Down
3 changes: 3 additions & 0 deletions src/amber/controller/helpers/render.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
require "kilt"
require "kilt/slang"

module Amber::Controller::Helpers
module Render
LAYOUT = "application.slang"
Expand Down