Rails core - Sep 2012 - Proposal: partials should not create a local variable with the name of the partial when no :local or :object option was passed

From this stack overflow
question<http://stackoverflow.com/questions/3111844/flash-messages-in-partials-rails-3/12482879>
.

Suppose I have a partial _flash.html.erb for rendering the flash in a 
consistent way. The natural thing to do would be to assume I can call 
ActionView::Base#flash from within this partial to retrieve the flash, and 
render the partial withrender partial: "flash". But in fact, this
doesn''t
work. Even though I haven''t passed object: flash orlocals: {flash:
flash} in
the call to render partial, PartialRenderer oh-so-helpfully defines a flashlocal
variable, whose value is nil, shadowing ActionView::Base#flash. My 
alternatives are to rename the partial or pass a value for the flash local 
explicitly using one of those more verbose call to render.

I suggest that if no object option or local with the same name as the 
partial was passed to the render call, thatPartialRenderer *not* define a 
local variable of the same name as the partial. This would change the 
behavior for anyone who is relying on the current behavior to get a nil value 
for the local when they don''t pass an explicit value, but that seems
like
an unlikely case and easily fixed by passing nil explicitly, and it would 
be much more useful to do things like call ActionView::Base#flash from 
within a _flash.html.erb partial.

I''m happy to create a pull request if others agree.

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To view this discussion on the web visit
https://groups.google.com/d/msg/rubyonrails-core/-/sHiSiBDAdOkJ.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

I''m -1 on ''not creating local variable'', but will +1
if you''re going to make it check first if there''s a method
with the same name defined or not.''

I can see that people will go outrage if we remove this feature …

Thanks!

- Prem  


On Tuesday, September 18, 2012 at 3:01 PM, John Firebaugh wrote:
>  
> From this stack overflow question
(http://stackoverflow.com/questions/3111844/flash-messages-in-partials-rails-3/12482879).
>  
>  
> Suppose I have a partial _flash.html.erb for rendering the flash in a
consistent way. The natural thing to do would be to assume I can call
ActionView::Base#flash from within this partial to retrieve the flash, and
render the partial withrender partial: "flash". But in fact, this
doesn''t work. Even though I haven''t passed object: flash
orlocals: {flash: flash} in the call to render partial, PartialRenderer
oh-so-helpfully defines a flashlocal variable, whose value is nil, shadowing
ActionView::Base#flash. My alternatives are to rename the partial or pass a
value for the flash local explicitly using one of those more verbose call to
render.
>  
>  
> I suggest that if no object option or local with the same name as the
partial was passed to the render call, thatPartialRenderer not define a local
variable of the same name as the partial. This would change the behavior for
anyone who is relying on the current behavior to get a nil value for the local
when they don''t pass an explicit value, but that seems like an unlikely
case and easily fixed by passing nil explicitly, and it would be much more
useful to do things like call ActionView::Base#flash from within a
_flash.html.erb partial.
>  
>  
> I''m happy to create a pull request if others agree.
>  
>  
> --  
> You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
> To view this discussion on the web visit
https://groups.google.com/d/msg/rubyonrails-core/-/sHiSiBDAdOkJ.
> To post to this group, send email to rubyonrails-core@googlegroups.com
(mailto:rubyonrails-core@googlegroups.com).
> To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
(mailto:rubyonrails-core+unsubscribe@googlegroups.com).
> For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On Tue, Sep 18, 2012 at 12:04 PM, Prem Sichanugrist
<sikandsak@gmail.com>wrote:
> I''m -1 on ''not creating local variable'', but
will +1 if you''re going to
> make it check first if there''s a method with the same name defined
or not.''
>
Check first and then do what? Not create the local variable in that case?

> I can see that people will go outrage if we remove this feature …
>
Are you aware of uses of relying on an implicitly nil local variable?

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On Tue, Sep 18, 2012 at 12:01:59PM -0700, John Firebaugh
wrote:
> 
> 
> From this stack overflow
question<http://stackoverflow.com/questions/3111844/flash-messages-in-partials-rails-3/12482879>
> .
> 
> Suppose I have a partial _flash.html.erb for rendering the flash in a 
> consistent way. The natural thing to do would be to assume I can call 
> ActionView::Base#flash from within this partial to retrieve the flash, and 
> render the partial withrender partial: "flash". But in fact, this
doesn''t
> work. Even though I haven''t passed object: flash orlocals: {flash:
flash} in
> the call to render partial, PartialRenderer oh-so-helpfully defines a
flashlocal
> variable, whose value is nil, shadowing ActionView::Base#flash. My 
> alternatives are to rename the partial or pass a value for the flash local 
> explicitly using one of those more verbose call to render.
> 
> I suggest that if no object option or local with the same name as the 
> partial was passed to the render call, thatPartialRenderer *not* define a 
> local variable of the same name as the partial. This would change the 
> behavior for anyone who is relying on the current behavior to get a nil
value
> for the local when they don''t pass an explicit value, but that
seems like
> an unlikely case and easily fixed by passing nil explicitly, and it would 
> be much more useful to do things like call ActionView::Base#flash from 
> within a _flash.html.erb partial.
> 
> I''m happy to create a pull request if others agree.
Do you know where the code is that does this?  I''d like to understand
why we do it before agreeing / disagreeing.

-- 
Aaron Patterson
http://tenderlovemaking.com/

Right, I think I understand the problem incorrectly the first time.

If we do `render ''flash''`, then there should be no local
variable name `flash` that is nil. Does Rails currently create a nil variable
name `flash`?

- Prem 


On Sunday, September 23, 2012 at 4:39 PM, Aaron Patterson wrote:
> On Tue, Sep 18, 2012 at 12:01:59PM -0700, John Firebaugh wrote:
> > 
> > 
> > From this stack overflow
question<http://stackoverflow.com/questions/3111844/flash-messages-in-partials-rails-3/12482879>
> > .
> > 
> > Suppose I have a partial _flash.html.erb for rendering the flash in a 
> > consistent way. The natural thing to do would be to assume I can call 
> > ActionView::Base#flash from within this partial to retrieve the flash,
and
> > render the partial withrender partial: "flash". But in fact,
this doesn''t
> > work. Even though I haven''t passed object: flash orlocals:
{flash: flash} in
> > the call to render partial, PartialRenderer oh-so-helpfully defines a
flashlocal
> > variable, whose value is nil, shadowing ActionView::Base#flash. My 
> > alternatives are to rename the partial or pass a value for the flash
local
> > explicitly using one of those more verbose call to render.
> > 
> > I suggest that if no object option or local with the same name as the 
> > partial was passed to the render call, thatPartialRenderer *not*
define a
> > local variable of the same name as the partial. This would change the 
> > behavior for anyone who is relying on the current behavior to get a
nil value
> > for the local when they don''t pass an explicit value, but
that seems like
> > an unlikely case and easily fixed by passing nil explicitly, and it
would
> > be much more useful to do things like call ActionView::Base#flash from
> > within a _flash.html.erb partial.
> > 
> > I''m happy to create a pull request if others agree.
> 
> Do you know where the code is that does this? I''d like to
understand
> why we do it before agreeing / disagreeing.
> 
> -- 
> Aaron Patterson
> http://tenderlovemaking.com/
> 
> 

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

Yes, similarly if you do render @customer you get a variable called
''customer'' when you render that template.

-- 
Cheers,

Koz


On Monday, 24 September 2012 at 10:07 AM, Prem Sichanugrist wrote:
> Right, I think I understand the problem incorrectly the first time.
> 
> If we do `render ''flash''`, then there should be no local
variable name `flash` that is nil. Does Rails currently create a nil variable
name `flash`?
> 
> - Prem 
> 
> On Sunday, September 23, 2012 at 4:39 PM, Aaron Patterson wrote:
> 
> > On Tue, Sep 18, 2012 at 12:01:59PM -0700, John Firebaugh wrote:
> > > 
> > > 
> > > From this stack overflow
question<http://stackoverflow.com/questions/3111844/flash-messages-in-partials-rails-3/12482879>
> > > .
> > > 
> > > Suppose I have a partial _flash.html.erb for rendering the flash
in a
> > > consistent way. The natural thing to do would be to assume I can
call
> > > ActionView::Base#flash from within this partial to retrieve the
flash, and
> > > render the partial withrender partial: "flash". But in
fact, this doesn''t
> > > work. Even though I haven''t passed object: flash
orlocals: {flash: flash} in
> > > the call to render partial, PartialRenderer oh-so-helpfully
defines a flashlocal
> > > variable, whose value is nil, shadowing ActionView::Base#flash.
My
> > > alternatives are to rename the partial or pass a value for the
flash local
> > > explicitly using one of those more verbose call to render.
> > > 
> > > I suggest that if no object option or local with the same name as
the
> > > partial was passed to the render call, thatPartialRenderer *not*
define a
> > > local variable of the same name as the partial. This would change
the
> > > behavior for anyone who is relying on the current behavior to get
a nil value
> > > for the local when they don''t pass an explicit value,
but that seems like
> > > an unlikely case and easily fixed by passing nil explicitly, and
it would
> > > be much more useful to do things like call ActionView::Base#flash
from
> > > within a _flash.html.erb partial.
> > > 
> > > I''m happy to create a pull request if others agree.
> > 
> > Do you know where the code is that does this? I''d like to
understand
> > why we do it before agreeing / disagreeing.
> > 
> > -- 
> > Aaron Patterson
> > http://tenderlovemaking.com/
> > 
> > 
> > 
> 
> 
> -- 
> You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
> To post to this group, send email to rubyonrails-core@googlegroups.com
(mailto:rubyonrails-core@googlegroups.com).
> To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
(mailto:rubyonrails-core+unsubscribe@googlegroups.com).
> For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On Mon, Sep 24, 2012 at 5:39 AM, Aaron Patterson
<tenderlove@ruby-lang.org> wrote:
> Do you know where the code is that does this?  I''d like to
understand
> why we do it before agreeing / disagreeing.
Around here, I guess
https://github.com/rails/rails/blob/780ecb2b/actionpack/lib/action_view/renderer/partial_renderer.rb#L300

I remember I once actually hit the same problem and wrote a patch like
this (but I forgot why I didn''t finish the patch and send a PR).
https://github.com/amatsuda/rails/commit/5b1c542ccb5a5aed6a2a9764c43523e103057336

-- 
Akira Matsuda<ronnie@dio.jp>

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.