From dc80fdf2bd1f134593422a24bbfd4d546108e61d Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sat, 7 Mar 2020 16:57:41 -0500 Subject: [PATCH 1/2] [Breaking change] generate getters for needs in pages Closes #853 The breaking change is that you can't use ? in 'needs' anymore. Instead we generate a 'getter?' if the type is a Bool. Also we may overwrite a method by generating a getter but I think there is very little change since most methods would be defined *after* needs --- spec/lucky/assignable_spec.cr | 12 +++++++++--- src/lucky/assignable.cr | 5 ++++- src/lucky/html_builder.cr | 15 ++++++++++++++- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/spec/lucky/assignable_spec.cr b/spec/lucky/assignable_spec.cr index 703892eed..6607a2369 100644 --- a/spec/lucky/assignable_spec.cr +++ b/spec/lucky/assignable_spec.cr @@ -36,10 +36,10 @@ end class PageWithQuestionMark include Lucky::HTMLPage - needs signed_in? : Bool + needs signed_in : Bool def render - text @signed_in.to_s + text signed_in?.to_s end end @@ -61,6 +61,11 @@ end class PageWithMetaclass include Lucky::HTMLPage needs string_class : String.class + needs access_me_with_a_getter : String = "called from an auto-generated getter" + + def render + text access_me_with_a_getter + end end describe "Assigns within multiple pages with the same name" do @@ -68,8 +73,9 @@ describe "Assigns within multiple pages with the same name" do PageOne.new build_context, title: "foo", name: "Paul", second: "second" PageTwo.new build_context, title: "foo", name: "Paul" PageThree.new build_context, name: "Paul", admin_name: "Pablo", title: "Admin" - PageWithQuestionMark.new(build_context, signed_in?: true).perform_render.to_s.should contain("true") + PageWithQuestionMark.new(build_context, signed_in: true).perform_render.to_s.should contain("true") PageWithDefaultsFirst.new(build_context, required: "thing", title: "foo").perform_render.to_s.should contain("special foo") PageWithMetaclass.new(build_context, string_class: String) + .perform_render.to_s.should contain("called from an auto-generated getter") end end diff --git a/src/lucky/assignable.cr b/src/lucky/assignable.cr index 9a0b14a98..948c23f93 100644 --- a/src/lucky/assignable.cr +++ b/src/lucky/assignable.cr @@ -3,7 +3,10 @@ module Lucky::Assignable macro needs(*type_declarations) {% for declaration in type_declarations %} {% unless declaration.is_a?(TypeDeclaration) %} - {% raise "needs expected a type declaration like 'name : String', instead got: '#{declaration}'" %} + {% raise "'needs' expects a type declaration like 'name : String', instead got: '#{declaration}'" %} + {% end %} + {% if declaration.var.stringify.ends_with?("?") %} + {% raise "Using '?' in a 'needs' var name is no longer supported. Now Lucky generates a method ending in '?' if the type is 'Bool'." %} {% end %} {% ASSIGNS << declaration %} {% end %} diff --git a/src/lucky/html_builder.cr b/src/lucky/html_builder.cr index d238aed12..2787bc3ec 100644 --- a/src/lucky/html_builder.cr +++ b/src/lucky/html_builder.cr @@ -28,6 +28,7 @@ module Lucky::HTMLBuilder macro setup_initializer_hook macro finished generate_needy_initializer + generate_getters end macro included @@ -61,7 +62,7 @@ module Lucky::HTMLBuilder {% value = declaration.value %} {% value = nil if type.stringify.ends_with?("Nil") && !value %} {% has_default = value || value == false || value == nil %} - {% if false || var.stringify.ends_with?("?") %}{{ var }}{% end %} @{{ var.stringify.gsub(/\?/, "").id }} : {{ type }}{% if has_default %} = {{ value }}{% end %}, + @{{ var.id }} : {{ type }}{% if has_default %} = {{ value }}{% end %}, {% end %} **unused_exposures ) @@ -69,6 +70,18 @@ module Lucky::HTMLBuilder {% end %} end + macro generate_getters + {% if !@type.abstract? %} + {% for declaration in ASSIGNS %} + {% if declaration.type.stringify == "Bool" %} + getter? {{ declaration }} + {% else %} + getter {{ declaration }} + {% end %} + {% end %} + {% end %} + end + def perform_render : IO render view From 9954b32446bf3b75765c4099b9c4c4372a80993f Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sat, 7 Mar 2020 17:16:14 -0500 Subject: [PATCH 2/2] Add docs to 'needs' --- src/lucky/assignable.cr | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/lucky/assignable.cr b/src/lucky/assignable.cr index 948c23f93..d77325fc3 100644 --- a/src/lucky/assignable.cr +++ b/src/lucky/assignable.cr @@ -1,5 +1,26 @@ -# :nodoc: module Lucky::Assignable + # Declare what a page needs in order to be initialized. + # + # This will declare an instance variable and getter automatically. It will + # also add arguments to an `initialize` method at the end of compilation. + # + # ### Examples + # + # ```crystal + # class Users::IndexPage < MainLayout + # # This page needs a `User` or it will fail to compile + # # You can access it with `@user` or the getter method `user` + # needs user : User + # + # # This page can take an optional `ProductQuery`. This means you can + # # Leave `products` off when rendering from an Action. + # needs products : ProductQuery? + # + # # When using a `Bool` Lucky will generate a method ending with `?` + # # So in this case you can call `should_show_sidebar?` in the page. + # needs should_show_sidebar : Bool = true + # end + # ``` macro needs(*type_declarations) {% for declaration in type_declarations %} {% unless declaration.is_a?(TypeDeclaration) %} @@ -12,6 +33,7 @@ module Lucky::Assignable {% end %} end + # :nodoc: macro included SETTINGS = {} of Nil => Nil ASSIGNS = [] of Nil @@ -25,6 +47,7 @@ module Lucky::Assignable end end + # :nodoc: macro inherit_page_settings SETTINGS = {} of Nil => Nil ASSIGNS = [] of Nil