-
Notifications
You must be signed in to change notification settings - Fork 15
Feature/liquid review templates loading #20
Conversation
lib/gyro/liquidgen/generator.rb
Outdated
@@ -24,18 +24,22 @@ module Liquidgen | |||
# | |||
class Generator | |||
|
|||
attr_accessor :params, :output_dir, :template_dir | |||
attr_accessor :params, :output_dir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done see 5ae90bf
filename_template_string = filename_template_path.readlines.first | ||
filename_template = Liquid::Template.parse(filename_template_string) | ||
filename_template.render(context).chomp | ||
@entity_filename_template.render(context).chomp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you removed/commented line 55 above, there's no more guarantee that the template use to provide the filename will render as a single line. In case the @*_filename_template.render(…)
generates more than one line, that's gonna generate ugly/strange file names in the Finder with newlines…
We should add some sort of security for those, like replacing all "\n"
by " "
for example, or only take the first line into account… (chomp
only remove the newline characters at the end of the string, not in the middle)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or better, emit a warning (or even error out) as we discussed yesterday, to tell the user to fix the entity_filename.liquid
template, because after all that's the template author's responsibility 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, see 9b62c02
enum_filename_template_string = enum_filename_template_path.readlines.first | ||
enum_filename_template = Liquid::Template.parse(enum_filename_template_string) | ||
enum_filename_template.render(context).chomp | ||
@enum_filename_template.render(context).chomp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, see 9b62c02
lib/gyro/liquidgen/generator.rb
Outdated
Gyro::Error.exit_with_error('Bad template directory content ! Your template need to ' + template_path.to_s + ' file') unless template_path.exist? | ||
template_path_string = template_path.read | ||
#filename_template_string = filename_template_path.readlines.first - filename.liquid | ||
Gyro::Log.error('The given template ' + template_path.to_s + ' contains return line(s). This can lead to side effets.') unless !(preventReturnLine && template_path_string.include?("\n")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer if x
to unless !x
Load templates in generator initializer once using private class method. ( see https://github.com/NijiDigital/gyro/projects/1#card-3129113 )