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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve Template#inspect output #35407

Merged
merged 3 commits into from
Feb 25, 2019
Merged

Improve Template#inspect output #35407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
675b21d
Don't call inspect from identifier_method_name
jhawthorn Feb 25, 2019
6afc4c2
Add locals Template#inspect
jhawthorn Feb 25, 2019
163b95d
Add tests for short_identifier and inspect
jhawthorn Feb 25, 2019
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
8 changes: 6 additions & 2 deletions actionview/lib/action_view/template.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,12 @@ def refresh(view)
end
end

def short_identifier
@short_identifier ||= defined?(Rails.root) ? identifier.sub("#{Rails.root}/", "") : identifier
Copy link
Contributor

Choose a reason for hiding this comment

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

I鈥檓 guessing what鈥檚 identifier and short_identifier here is what鈥檚 path(?) and virtual_path elsewhere in Action View. Might be nice if the naming was unified for our internals. Though that鈥檚 another PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think they are a little different. virtual_path is relative to the view directory (ex. app/views), where short_identifier is relative to the app root (or from / if from a gem outside). I think it's also possible for identifier not to be from the FS if the template comes from some other source.

I do agree that it does feel like something here could be unified though!

end

def inspect
@inspect ||= defined?(Rails.root) ? identifier.sub("#{Rails.root}/", "") : identifier
"#<#{self.class.name} #{short_identifier} locals=#{@locals.inspect}>"
end

# This method is responsible for properly setting the encoding of the
Expand Down Expand Up @@ -377,7 +381,7 @@ def method_name
end

def identifier_method_name
inspect.tr("^a-z_", "_")
short_identifier.tr("^a-z_", "_")
end

def instrument(action, &block) # :doc:
Expand Down
10 changes: 10 additions & 0 deletions actionview/test/template/template_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,4 +215,14 @@ def with_external_encoding(encoding)
ensure
silence_warnings { Encoding.default_external = old }
end

def test_short_identifier
@template = new_template("hello")
assert_equal "hello template", @template.short_identifier
end

def test_template_inspect
@template = new_template("hello")
assert_equal "#<ActionView::Template hello template locals=[]>", @template.inspect
end
end