-
Notifications
You must be signed in to change notification settings - Fork 42
Should InlineWidget override capture? #23
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
Comments
I'm going to have to look at this more carefully when I'm smarter :-) |
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? |
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. |
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
The text was updated successfully, but these errors were encountered: