Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should InlineWidget override capture? #23

Open
maca opened this issue Oct 1, 2011 · 3 comments
Open

Should InlineWidget override capture? #23

maca opened this issue Oct 1, 2011 · 3 comments

Comments

@maca
Copy link

maca commented Oct 1, 2011

Hi, I've just made some changes in Tilt to support erector templates. I struggled a bit with using yield from the template, using #capture from an InlineWidget passing a block defined outside the widget would evaluate the block in the context where it was originally declared, maybe InliteWidget should override capture evaluating the block with instance_eval instead of yield.

Here's my commit to tilt, I overrode capture:

maca/tilt@a5776e5

@alexch
Copy link
Member

alexch commented Oct 1, 2011

I'm going to have to look at this more carefully when I'm smarter :-)

@alexch
Copy link
Member

alexch commented Oct 1, 2011

I took a stab at that the other day too but I didn't get as far as you. I think there's more to do to your code before it's a pull-request-worthy patch -- e.g. documentation -- but I'm glad you took the initiative.

I see that you have enabled ".erector" view files, which are like Erector classes without the "class Foo < Erector::Widget; def content" at the top, which means Erector features like "needs" and "externals" are not available, not to mention the ability to extract functions. In Rails we used ".rb" files, which allowed real classes, but also led to some weirdness around forcing the class name to conform to the file and path.

Maybe a .erector file should have an implied class, but not an implied content method, so this:

views/foo/bar.erector:

needs :name
def content
  h1 @name
end

would be the same as this:

views/foo/bar.rb

class Views::Foo::Bar < Erector::Widget
  needs :name
  def content
    h1 @name
  end
end

The issue of load path munging also needs to be addressed, since the Rails system requires the "app" directory to be in the load path and I've heard some grumbling that that's a security problem, though I'm not sure what scenario they're afraid of. But how else will we let widgets call other widgets?

@maca
Copy link
Author

maca commented Oct 1, 2011

I took a start from the MarkabyTemplate from Tilt, it basically creates an anonymous class that inherits from Erector::Widget and defines a method from the template content.

I am perfectly happy for the template being just the html generation dsl bits and being evaluated in the context of an already defined widget, but I can see it misses some of the Widget niceties.

Well, I guess the class name can be inferred from the template file name and then autoload could be used, that imply that subsequent renders of the template wouldn't even need to read the file because the class is already loaded but templates would not reload on each request in development. I think I can give it a shot but I also like the "naked inline dsl template" because I would like to mix erector with other templating languages such as slim and use it for forms and helpers and also subscribe to the convention of using yield for rendering templates inside layouts.

If I where to make a sinatra app (or a Rails app) that would go with Erector all the way I would load erector clases just like any other class and I would instantiate the widget and call to_html in the response block, what I mean is that I think Erector::Widget(s) can do perfectly well without Tilt. There may be some advantages I am missing of using tilt to render widget classes.

Personally I would like to have both options, rendering "inline" templates and widget classes, that would imply two tilt extensions and different file extensions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants