Masterview devel - Oct 2006 - It appears that we need to enclose our method calls in parens so ERB can parse complex methods - Re: Safer code in rendered output

On 10/10/06, Jeff Barczewski <jeff.barczewski at gmail.com>
wrote:
> On 10/9/06, Ed Howland <ed.howland at gmail.com> wrote:
> > When rendering the directives into tags, you should almost always use
> > the ''fn(p1, p2)'' form rather than the ''fn 
p1, p2'' style. This is
> > because you can''t predict what extra options the users are
going to
> > put on the mv: directives. W/o doing this, some ERb stuff is going to
> > barf in Ruby because eventually, it all gets compiled down to a call
> > to:
> >  _erbout(  .... )
> >
> > Passing multiple hashes to mv: directives like mv:form in order to put
> > html_options will cause this
> > problem.
> >
> > Note: You can get around the problem by explicitly parenthesizing
> > stuff like this:
> >
> > <form .... mv:form="({:action =>
''chart''},  {:name => ''chart''})">
> >
> > It may look prettier in the rendered output, but you should always opt
> > for the safer output.
> >
> > Ed
> > --
>
> We do test the ability to pass in multiple hashes with the directives
> but you are right that it can get tricky depending on the scenario.
>
> I did have some directive tests for multiple hashes (link_to) and
> others so we were trying to make sure that they worked.
>
> However let me go back and review those test cases and also make sure
> that erb was working with the output of those as well. Sounds like you
> for sure hit a situation when it didn''t.
>
> If that is the case then it is fairly easy change to change to
> method() style output with the new refactoring. And I think you are
> right in that it is safer (easier for the parser to interpret).
>
> Thanks for the heads up.
>
Deb,

I have confirmed Ed''s finding that erb cannot handle parsing of
form_tag with multiple hashes without the surrounding parens. So I
guess the best solution is for the directives to render parens around
the arguments. This is a simple fix in the new code since
prepare_output gets all the arguments ready for erb_content, however I
will have to go through and update a bunch of test cases.

Let me know if you see any reason why we shouldn''t always put parens
around directive generated methods (to help out ERB). I should be able
to go through and make the changes tomorrow morning.

PS. rubyforge appears to be down so, copy me directly on this reply so
I get it sooner.

Jeff

Jeff - I should probably go through some of the code and some examples
again, but I think I agree with Ed that our directive implementations should
be emitting parenthesized fcn call notation.

I really don''t see any downside.  It''s slightly less
declarative looking in
simple cases, but then this is generated code and correctness is more
important.

There are cases where the paren-less function notation permitted by Ruby is
quite nice (all the DSL-style, declarative notations that it enabled), but I
certainly find for my own coding style that are many situations where the
more traditional notation with parens seems more approprate.

While our generated code might use parens that perhaps you''d have
omitted if
writing the equivalent .rhtml by hand, I think anyone reading the output
will not be confused by the paren notation.

Nobody should *ever* have to write the sort of markup Ed described:
> > Note: You can get around the problem by explicitly parenthesizing 
> > stuff like this:
> >
> > <form .... mv:form="({:action =>
''chart''},  {:name => ''chart''})">
> >
>
So I''d vote to proceed with this.  Changing/improving tests cases as a
side
effect is a good thing.

[all yours, I''m not in any of that code right now.  Just working on
finishing up some final sections of the developer''s documentation for
the
new directive implementation mechanism]

~ Deb

On 10/12/06, Deb Lewis <djlewis at acm.org> wrote:
> Jeff - I should probably go through some of the code and some examples
> again, but I think I agree with Ed that our directive implementations
should
> be emitting parenthesized fcn call notation.
>
> I really don''t see any downside.  It''s slightly less
declarative looking in
> simple cases, but then this is generated code and correctness is more
> important.
>
> There are cases where the paren-less function notation permitted by Ruby is
> quite nice (all the DSL-style, declarative notations that it enabled), but
I
> certainly find for my own coding style that are many situations where the
> more traditional notation with parens seems more approprate.
>
> While our generated code might use parens that perhaps you''d have
omitted if
> writing the equivalent .rhtml by hand, I think anyone reading the output
> will not be confused by the paren notation.
>
> Nobody should *ever* have to write the sort of markup Ed described:
>
I fully agree. Just wanted to make sure before I changed all the test cases :-)

Thanks again Ed for bringing this to our attention.