Fix partial rendering with dot in filename #24052
Conversation
When rendering a collection with a partial whose filename contains a dot, e.g. "customer.mobile", we would set a `locals[:'customer.mobile']` variable instead of, as in earlier versions of Rails, `locals[:customer]`. This bug was introduced in da9038e.
|
r? @schneems (@rails-bot has picked a reviewer for you, use r? to override) |
|
Do you know why or when this behavior changed? |
|
@amatsuda can you take a look at this PR? |
|
@bquorning Interesting. I've never seen such a code, and it's actually an untested (=undefined) behavior so far, but there's no reason not to keep the compatibility on your code. |
…ction-and-dot-in-partial-name Fix partial rendering with dot in filename
bquorning
deleted the
partial-rendering-with-collection-and-dot-in-partial-name
branch
|
Thank you for the merge. |
| @@ -521,7 +521,7 @@ def retrieve_template_keys | |||
| def retrieve_variable(path, as) | |||
| variable = as || begin | |||
| base = path[-1] == "/".freeze ? "".freeze : File.basename(path) | |||
| raise_invalid_identifier(path) unless base =~ /\A_?(.*)(?:\.\w+)*\z/ | |||
| raise_invalid_identifier(path) unless base =~ /\A_?(.*?)(\.\w+)*\z/ | |||
There was a problem hiding this comment.
Ah, sorry. I just now noticed that there’s really no need to remove ?: from the last regex capture.
There was a problem hiding this comment.
True. I added the ?: to express that we're only using $1 afterwards.
It would work without ?:, or you can make another PR adding ?:, or I would just push these 2 chars.
I'm fine with any option :)
There was a problem hiding this comment.
You can commit directly to master – that’s probably the easiest option :-)
There was a problem hiding this comment.
@bquorning Done. 28a6571
Thanks!
When rendering a collection with a partial whose filename contains a dot, e.g. "customer.mobile", we would set a
locals[:'customer.mobile']variable instead of, as in earlier versions of Rails,locals[:customer].This bug was introduced in da9038e.