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

Removed Class Eval and used define_method instead for the SafeBuffer #10600

Merged
merged 2 commits into from May 14, 2013
Merged
Changes from 1 commit
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
21 changes: 9 additions & 12 deletions activesupport/lib/active_support/core_ext/string/output_safety.rb
Expand Up @@ -171,18 +171,15 @@ def encode_with(coder)
end

UNSAFE_STRING_METHODS.each do |unsafe_method|
if 'String'.respond_to?(unsafe_method)
class_eval <<-EOT, __FILE__, __LINE__ + 1
def #{unsafe_method}(*args, &block) # def capitalize(*args, &block)
to_str.#{unsafe_method}(*args, &block) # to_str.capitalize(*args, &block)
end # end

def #{unsafe_method}!(*args) # def capitalize!(*args)
@html_safe = false # @html_safe = false
super # super
end # end
EOT
end
if String.new.respond_to?(unsafe_method)
define_method(unsafe_method.to_sym) do |*args, &block|
Copy link
Member

Choose a reason for hiding this comment

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

no need to call to_sym

Copy link
Member

Choose a reason for hiding this comment

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

Could you remove this to_sym call? Is it necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK..removing to_sym method 👍

to_str.send(unsafe_method, *args, &block)
end
Copy link
Member

Choose a reason for hiding this comment

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

Missing blank line after this

Copy link
Member

Choose a reason for hiding this comment

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

A blank line here would make the things clear

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where??? between two define_methods???

define_method("#{unsafe_method}!".to_sym) do |*args|
@html_safe = false
super(*args)
Copy link
Member

Choose a reason for hiding this comment

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

No need to pass the args

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would throw up an error if we don't pass in the args to the super function

Choose a reason for hiding this comment

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

Yeah, super with no args doesn't work from define_method =/

end
end
end
end
end
Expand Down