Improve Template#inspect output#35407
Conversation
Handler, formats, and variant are usually obvious from looking at the identifier. However it's not uncommon to have different locals for the same template so we should make that obvious in inspect.
kaspth
left a comment
There was a problem hiding this comment.
Do we print the inspect output in logs or the log subscriber? Could be good to double check what a rendering log output looks like.
| end | ||
|
|
||
| def short_identifier | ||
| @short_identifier ||= defined?(Rails.root) ? identifier.sub("#{Rails.root}/", "") : identifier |
There was a problem hiding this comment.
I’m guessing what’s identifier and short_identifier here is what’s path(?) and virtual_path elsewhere in Action View. Might be nice if the naming was unified for our internals. Though that’s another PR.
There was a problem hiding this comment.
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!
👍 Good idea. Logging looks unchanged in my test app. I also tested an exception from a view and it looked normal.
|
|
Gotcha! Thanks for checking both. |
This is a quality of life improvement for those developing ActionView itself (cc @tenderlove)
This PR first renames the existing
inspectimplementation toshort_identifier. This improves the 👀 reading ofidentifier_method_name, which used to readinspect.tr("^a-z_", "_"), which is a bit awkward because it relies on whatinspecthappens to return.short_identifier.tr("^a-z_", "_")conveys what it does better (and allows us to changeinspect🎉 ).Inspect used to return a string like
app/views/home/index.html.erb, which is confusing because I would normally expect to see a type and it isn't immediately obvious that that isn't a string, particularly when in an array (ie.[some/template, another/template]parses to me the same as["some/template", "another/template"]on first glance).This PR changes inspect to return a string like:
Including the class name makes it look more like most
#inspectmethods. I also addedlocalsto this output because it's often different for the same identifier (we need to compile each set of locals separately). It could be helpful to include handler, formats, and variants, but they should be inferable by the developer from the identifier, so I didn't include them.I'd guess the reason we override inspect here in the first place is to avoid printing out the whole source code for the template.