Doh. 'spose I shoulda tested it. Especially since that was the first
time I'd ever used inject. I feel like an ass now.

Thanks for getting that in so quick; I post and the next time I update
the fix is in.

I'll keep my eyes out for anything that looks like an easy fix, and
post up if I find anything interesting.

I've actually been kinda interested in modifying the Engine through
redefining certain methods; it would be nice if the compile and render
methods could be factored into sub-methods (like an assign_locals
method, in this case), to allow for easier overrides. If you're gonna
be reworking it pretty significantly for 2.0, I won't bother messing
with it now.

I'll definitely be looking forward to performance improvements, though
I've been pleasantly surprised by the current implementation (I'd read
in a few places that Haml was a lot slower than ERB).

Nick

On Apr 15, 9:26 pm, Nathan Weizenbaum <[EMAIL PROTECTED]> wrote:
> That actually breaks... k ends up with a two element key/value array and
> v ends up with nil. Also, inject takes the return value of the last
> block as the memo value for the next block, so you'd have to do
>
> @scope_object.extend(@options[:locals].inject(Module.new) { |m, k| 
> m.send(:define_method, k[0]) { k[1] }; m })
>
> As to the messiness of the codebase, I think to some extent it's
> inevitable. As we add more features, write more stuff to handle edge
> cases, etc. it's not always possible to keep it elegant. Not to mention
> that, as humans, we tend to make stupid mistakes from time to time (like
> this one... heh).
>
> As part of the 2.0 release, I'm planning to do some serious reworking of
> the Engine/Buffer rendering system. This is primarily to increase
> efficiency by making as much rendering as possible happen before the
> template is cached, but it'll also offer a good opportunity to take a
> good look at the code that's currently doing the rendering and clean it
> up as much as possible.
>
> That's not to say that it wouldn't be worthwhile to do some cleaning
> now, though. If you're interested, by all means go for it.
>
> - Nathan
>
> Nick Howell wrote:
> > Or, if you want an uber-one-liner:
>
> > @scope_object.send(:extend, @options[:locals].inject(Module.new) { |m,
> > k, v| m.send(:define_method, k) { v } })
>
> > Is it just me, or could Haml use some code-cleanup?
>
> > Nick
>
> > On Apr 12, 6:34 pm, "Nathan Weizenbaum" <[EMAIL PROTECTED]> wrote:
>
> >> You're right, it should be like that. I don't know how it snuck through.
> >> I'll fix it later today.
>
> >> - Nathan
>
> >> On 4/12/07, Nick Howell <[EMAIL PROTECTED]> wrote:
>
> >>> In Haml::Engine#render:
> >>> <pre>
> >>> 176     def render(scope = Object.new, &block)
> >>> 177       @scope_object = scope
> >>> 178       @buffer = Haml::Buffer.new(@options)
> >>> 179
> >>> 180       local_assigns = @options[:locals]
> >>> 181
> >>> 182       # Get inside the view object's world
> >>> 183       @scope_object.instance_eval do
> >>> 184         # Set all the local assigns
> >>> 185         local_assigns.each do |key,val|
> >>> 186           self.class.send(:define_method, key) { val }
> >>> 187         end
> >>> 188       end
> >>> </pre>
>
> >>> Notice that scope and @scope_object are, by default, Objects. When
> >>> local_assigns are...assigned...the Object class has new methods
> >>> defined on it. This is *bad*.
>
> >>> I suggest that we replace it with this:
> >>> <pre>
> >>> m = Module.new
> >>> locals_assigns.each { |k, v| m.send(:define_method, k) { m } }
> >>> @scope_object.send(:extend, m)
> >>> </pre>
>
> >>> Thoughts?
>
> >>> Nick


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Haml" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/haml?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to