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

No nil format on templates #35404

Merged
merged 4 commits into from
Feb 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion actionpack/test/dispatch/debug_exceptions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def raise_nested_exceptions
def call(env)
env["action_dispatch.show_detailed_exceptions"] = @detailed
req = ActionDispatch::Request.new(env)
template = ActionView::Template.new(File.read(__FILE__), __FILE__, ActionView::Template::Handlers::Raw.new, {})
template = ActionView::Template.new(File.read(__FILE__), __FILE__, ActionView::Template::Handlers::Raw.new, format: :html)

case req.path
when "/pass"
Expand Down
17 changes: 12 additions & 5 deletions actionview/lib/action_view/template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,17 @@ def self.finalize_compiled_template_methods=(_)

extend Template::Handlers

attr_accessor :locals, :formats, :variants, :virtual_path
attr_accessor :locals, :variants, :virtual_path

attr_reader :source, :identifier, :handler, :original_encoding, :updated_at

attr_reader :variable
attr_reader :variable, :formats

def initialize(source, identifier, handler, details)
format = details[:format] || (handler.default_format if handler.respond_to?(:default_format))
def initialize(source, identifier, handler, format: nil, **details)
unless format
ActiveSupport::Deprecation.warn "ActionView::Template#initialize requires a format parameter"
format = :html
end

@source = source
@identifier = identifier
Expand All @@ -146,11 +149,15 @@ def initialize(source, identifier, handler, details)
end

@updated_at = details[:updated_at] || Time.now
@formats = Array(format).map { |f| f.respond_to?(:ref) ? f.ref : f }
@formats = Array(format)
@variants = [details[:variant]]
@compile_mutex = Mutex.new
end

def formats=(_)
end
deprecate :formats=

# Returns whether the underlying handler supports streaming. If so,
# a streaming buffer *may* be passed when it starts rendering.
def supports_streaming?
Expand Down
18 changes: 13 additions & 5 deletions actionview/lib/action_view/template/resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,6 @@ def decorate(templates, path_info, details, locals)
cached = nil
templates.each do |t|
t.locals = locals
t.formats = details[:formats] || [:html] if t.formats.empty?
t.variants = details[:variants] || [] if t.variants.empty?
t.virtual_path ||= (cached ||= build_path(*path_info))
end
Expand Down Expand Up @@ -225,7 +224,7 @@ def query(path, details, formats, outside_app_allowed)
template_paths = reject_files_external_to_app(template_paths) unless outside_app_allowed

template_paths.map do |template|
handler, format, variant = extract_handler_and_format_and_variant(template)
handler, format, variant = extract_handler_and_format_and_variant(template, formats.first)

FileTemplate.new(File.expand_path(template), handler,
virtual_path: path.virtual,
Expand Down Expand Up @@ -292,17 +291,26 @@ def mtime(p)
# Extract handler, formats and variant from path. If a format cannot be found neither
# from the path, or the handler, we should return the array of formats given
# to the resolver.
def extract_handler_and_format_and_variant(path)
def extract_handler_and_format_and_variant(path, query_format)
pieces = File.basename(path).split(".")
pieces.shift

extension = pieces.pop

handler = Template.handler_for_extension(extension)
format, variant = pieces.last.split(EXTENSIONS[:variants], 2) if pieces.last
format &&= Template::Types[format]
format = if format
Template::Types[format]&.ref
else
if handler.respond_to?(:default_format) # default_format can return nil
handler.default_format
else
query_format
end
end

[handler, format, variant]
# Template::Types[format] and handler.default_format can return nil
[handler, format || query_format, variant]
end
end

Expand Down
4 changes: 2 additions & 2 deletions actionview/lib/action_view/testing/resolvers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def query(path, exts, _, _)
@hash.each do |_path, array|
source, updated_at = array
next unless query.match?(_path)
handler, format, variant = extract_handler_and_format_and_variant(_path)
handler, format, variant = extract_handler_and_format_and_variant(_path, :html)
templates << Template.new(source, _path, handler,
virtual_path: path.virtual,
format: format,
Expand All @@ -49,7 +49,7 @@ def query(path, exts, _, _)

class NullResolver < PathResolver
def query(path, exts, _, _)
handler, format, variant = extract_handler_and_format_and_variant(path)
handler, format, variant = extract_handler_and_format_and_variant(path, :html)
[ActionView::Template.new("Template generated by Null Resolver", path.virtual, handler, virtual_path: path.virtual, format: format, variant: variant)]
end
end
Expand Down
2 changes: 1 addition & 1 deletion actionview/test/abstract_unit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def render_erb(string)
string.strip,
"test template",
ActionView::Template.handler_for_extension(:erb),
{})
format: :html)

view = ActionView::Base.with_empty_template_cache
template.render(view.empty, {}).strip
Expand Down
6 changes: 4 additions & 2 deletions actionview/test/template/template_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ def partial
"<%= @virtual_path %>",
"partial",
ERBHandler,
virtual_path: "partial"
virtual_path: "partial",
format: :html
)
end

Expand All @@ -55,7 +56,8 @@ def my_buffer
end
end

def new_template(body = "<%= hello %>", details = { format: :html })
def new_template(body = "<%= hello %>", details = {})
details = { format: :html }.merge details
ActionView::Template.new(body.dup, "hello template", details.fetch(:handler) { ERBHandler }, { virtual_path: "hello" }.merge!(details))
end

Expand Down