Jeff Barczewski
2006-Jul-31 21:02 UTC
[Masterview-devel] Revised and simplified directive base - would like your input on syntax and approach as soon as possible
To implement this DSL I created some powerful methods arg(instance_var_name, options) - gets a subargument out of the attr_value, shifting that subarg off the array so that subsequent calls will get the next subarg. Peform quoting or merging with html attributes. Define optional_default which will be used if the value is empty but needed when building a method call and post params are non-null. Stores this to an instance_var_name. erb_content and erb_eval can now take a string and a *args and it contains all the logic to build up the appropriate erb calls. I also added a method render which can be used to easily allow the developer to package up multiple outputs from a method. So rather than building up an array themselves and returning it, they can simply call render as many times as necessary and it will return the complete collection of outputs. The only gotcha is that it needs to be called as the last method in the event (so that the results are returned from the event). (This avoided us having to go to a more complicated block dsl which I think has its own issues, but is this too risky or can it be handled with simple documentation and examples). For completeness I added a method called render_nothing which returns nil, this is used when for the events that don''t want to render anything. I added a method called continue which calls the dcs.render which allows continuation of rendering down the directive call stack. So if you want to do this and to output it then you can do ''render continue'' or of course set the results to a variable and manipulate. The resulting syntax looks like this def etag(dcs) arg :obj, :quote => true arg :method, :quote => true arg :foo, { ''override with this value''} arg :bar, { |x| x.downcase } # lowercasing the arg before setting it to @bar arg :options, :merge => [:common_html, :size, :maxlength] render erb_content ''text_field'', :obj, :method, :options render continue render erb_eval ''end'' end To access the full directive attribute value use attr_value To access or set element attributes, use attr[:class] or attr[''class''] To access content of element, use content to get array, or content_str to get joined version How does this look to you? It really cleans things up considerably from the old code and should be much more foolproof to user not understanding the complexities. Are my naming conventions ok? (arg, render, continue, :quote, :merge, :optional_default) Once this is in place I would like to get rid of the old cludgy methods that are no longer needed in directive_base and directive_helpers. However what is the best approach to doing this? Do I make these methods in a new base class SimplifiedDirectiveBase? Does it inherit from DirectiveBase? or do I add them directly to DirectiveBase? Do I try to remove the methods that I don''t want in there after making sure nothing in our system or any submitted directives are using? Not sure the best approach to cleaning this up. I don''t want to leave around all those redundant methods which are not fully debugged and are tricky to use properly. I want to have a clean minimal set of methods that make it clear how to operate. Obviously we will document whatever we do and increment version when we make such an api jump. Need your input on this as soon as you get a chance since I am just about ready to rewrite the directives using the new api once you confirm the naming and the upgrade approach. Or if you think that we really need to move towards a block dsl (to get around the need for render to be last call in method) then I''ll have to work on that (using these method calls to get us most of the way there was really easy). Thanks, Jeff
Deb Lewis
2006-Aug-01 18:06 UTC
[Masterview-devel] Revised and simplified directive base - would like your input on syntax and approach as soon as possible
Jeff - I like the general direction, not entirely convinced about some of the specifics yet in the initial proposal. I''ll do a little brain-dumping and thinking-through-email here. Background context: my mental model of template and directive processing (and how I''ve been writing the developer document so far) is essentially that of XSLT - what''s happening is a tree transformation process on a source DOM tree from the xhtml doc with mv markup into a target rhtml doc with html and erb markup. The default transformation of an element in the source tree is to simply copy the element and its attributes to the destination tree. Directive attribute markup allows for modifying or overriding this default transformation process. A directive is an element attribute with a (string) value containing some encoding of its processing instructions. It''s instantiated at the start of the element to which it''s attached, with its (string) attribute value provided in the constructor; it can perform processing at element start and/or element end, with access to the source DOM via the dcs arg to stag/etag methods. Processing may involve modifications to the source DOM state (say, add/change/delete attribute values), with normal transformation rendering contining thereafter, or the directive implementation can take control of the output rendering of its element. Two thoughts relative to your arg() service. First, I think I''d like to generally think of the attribute value along similar lines as an args list to a ruby method: one or more comma separated values, which are the positional args, optionally with some trailing set of key => value pairs that get collected into a hash. With sort of a yaml-ish interpretation, though, so that you don''t have to be strict about wrapping quotes in the markup to get a string literal, etc. (?maybe your :quote => true option could be dropped? superfluous if we can just "do the right thing" for strings and numbers and then allow the block processing arg from client to supplement) Second, the arg markup done w/in the method implementation seems quite cryptic to me and somewhat inappropriate - auto-stuffing inst vars may be overkill, why not return the value and let me decide on temp vs. inst var w/in my method impl, and actually the declarative-style notation at that point just seems a bit odd. [side remark: i''m a bit skeptical about the current passion to label any and all form of framework and library protocol design as "a DSL", thereby attributing Goodness </rant>] But if you shift the placement of the arg service into the class declaration rather than w/in the s/etag processing method then it starts to feel better to me. Then it *is* a more declarative style notation - it describes the signature of the directive attribute: I expect some number of positional args and for each I can state processing options (:quote) or default value (:default => ''foo''). Then allow last attr_arg to collect a hash - your :merge option name seems ok for that. Such an attr_arg method implementation in DirectiveBase takes the declarations and builds a spec that''s used when the directive is instantiated to process the attribute value string into useful form, and then it *does* make sense to be popping the arg values off and putting them in inst vars for the processing method(s) in the directive implementation to access. class MySpecialDirective < DirectiveBase attr_arg :obj, :quote => true attr_arg :method, :quote => true attr_arg :foo, { ''override with this value''} attr_arg :bar, { |x| x.downcase } # lowercasing the arg before setting it to @bar attr_arg :options, :merge => [:common_html, :size, :maxlength] def etag(dcs) ...do some init or munging of the source DOM here... end def etag(dcs) ...output rendering stuff here... end end>> To access the full directive attribute value use attr_value >> To access or set element attributes, use attr[:class] or attr[''class''] >> To access content of element, use content to get array, or content_str toget joined version I''d say just go with the symbol version of the attr service: attr[:class], don''t bother with attr[''class''] variation. For accessing element content, maybe better name is element_content and element_content_str. And element_content should return the form that you normally want, which I think might be the string content. I need to think a bit more about the rendering methods portions of the proposal; I still need to wrap my brain around the dcs value that''s provided as the argument to stag and etag methods in a directive implementation. What I want when I''m writing a method in a directive implementation is clean protocol for accessing the source DOM - access or manipulate the element I''m attached to and its attributes, which I think you''ve almost got now. And then I want simple operations which specify the effect I want to produce on the output tree (with default being the copy-to-dest behavior of SimpleRenderHandler if I do nothing to specify rendering output) I''m not sure if it makes sense to break up SimpleRenderHandler and recast it as a mixin-module (StandardRenderHandler or somesuch) which establishes the default behavior - include in DirectiveBase and the backstop SimpleRenderHandler in parser.rb. RE: render_nothing - could/should this be done as a declarative statement on the directive class? RE: render continue - think I''d like something closer to the controller/view style behavior. If the directive doesn''t explicitly invoke a render operation to specify what it wants to produce in the output, then the default is to do the standard copy-src-to-dst behavior. RE: transition/upgrade strategy - I''m inclined to say this is a good time to just break directive implementations and "do it right" - get rid of the old junk, or maybe keep existing guy as as a separate (renamed) transition upgrade class but *ONLY* if a poll of our existing MV users indicates they really want and need this. I just don''t think there''s enough of an installed base in place to muddle the path forward with backwards compat. If I have directive impls at this early stage in MV''s life cycle, I think I''d want to get them cleaned up and simplified. So let''s work this out and send out an advance warning notice on the mailing lists, if there''s major upset then we can revisit. ~ Deb -----Original Message----- From: Jeff Barczewski [mailto:jeff.barczewski at gmail.com] Sent: Monday, July 31, 2006 2:02 PM To: masterview-devel at rubyforge.org; djlewis at acm.org Subject: Re: Revised and simplified directive base - would like your input on syntax and approach as soon as possible To implement this DSL I created some powerful methods arg(instance_var_name, options) - gets a subargument out of the attr_value, shifting that subarg off the array so that subsequent calls will get the next subarg. Peform quoting or merging with html attributes. Define optional_default which will be used if the value is empty but needed when building a method call and post params are non-null. Stores this to an instance_var_name. erb_content and erb_eval can now take a string and a *args and it contains all the logic to build up the appropriate erb calls. I also added a method render which can be used to easily allow the developer to package up multiple outputs from a method. So rather than building up an array themselves and returning it, they can simply call render as many times as necessary and it will return the complete collection of outputs. The only gotcha is that it needs to be called as the last method in the event (so that the results are returned from the event). (This avoided us having to go to a more complicated block dsl which I think has its own issues, but is this too risky or can it be handled with simple documentation and examples). For completeness I added a method called render_nothing which returns nil, this is used when for the events that don''t want to render anything. I added a method called continue which calls the dcs.render which allows continuation of rendering down the directive call stack. So if you want to do this and to output it then you can do ''render continue'' or of course set the results to a variable and manipulate. The resulting syntax looks like this def etag(dcs) arg :obj, :quote => true arg :method, :quote => true arg :foo, { ''override with this value''} arg :bar, { |x| x.downcase } # lowercasing the arg before setting it to @bar arg :options, :merge => [:common_html, :size, :maxlength] render erb_content ''text_field'', :obj, :method, :options render continue render erb_eval ''end'' end To access the full directive attribute value use attr_value To access or set element attributes, use attr[:class] or attr[''class''] To access content of element, use content to get array, or content_str to get joined version How does this look to you? It really cleans things up considerably from the old code and should be much more foolproof to user not understanding the complexities. Are my naming conventions ok? (arg, render, continue, :quote, :merge, :optional_default) Once this is in place I would like to get rid of the old cludgy methods that are no longer needed in directive_base and directive_helpers. However what is the best approach to doing this? Do I make these methods in a new base class SimplifiedDirectiveBase? Does it inherit from DirectiveBase? or do I add them directly to DirectiveBase? Do I try to remove the methods that I don''t want in there after making sure nothing in our system or any submitted directives are using? Not sure the best approach to cleaning this up. I don''t want to leave around all those redundant methods which are not fully debugged and are tricky to use properly. I want to have a clean minimal set of methods that make it clear how to operate. Obviously we will document whatever we do and increment version when we make such an api jump. Need your input on this as soon as you get a chance since I am just about ready to rewrite the directives using the new api once you confirm the naming and the upgrade approach. Or if you think that we really need to move towards a block dsl (to get around the need for render to be last call in method) then I''ll have to work on that (using these method calls to get us most of the way there was really easy). Thanks, Jeff
Jeff Barczewski
2006-Aug-01 18:58 UTC
[Masterview-devel] Revised and simplified directive base - would like your input on syntax and approach as soon as possible
Thanks for the feedback, that helps. I think it is helping to flush out the best approach here. I think your idea about moving the attr_arg to the class level definition make sense it should be at that level. I did just get through coding up a CaseInsensitiveHash that allows you to access values using symbol or string and it will work case independent. It will preserve the case though since that can be important for attributes. If you are setting a value for the first time then you may want to specify the case, subsequent access or even resetting a value keeps the original case. I think this will work well for us in the way we are using attributes. I agree with your assesment on rendering, it probably should work more like rails. I will try to think through some scenarios and see what I can come up with. I think to accomplish what we are wanting I will need to go to a block DSL syntax so that I can do the post processing we need. That gives us lots of flexibility for the future too (we can do any amount of pre or post processing). Some brainstorming and thinking out loud here... So syntax might be something like class MySpecialDirective < DirectiveBase attr_arg :obj, :quote => true attr_arg :method, :quote => true attr_arg :foo, { ''override with this value''} attr_arg :bar, { |x| x.downcase } # lowercasing the arg before setting it to @bar attr_arg :options, :merge => [:common_html, :size, :maxlength] event :stag do |e| e.render ''something here'' e.render erb_content( ''text_area'', :obj, :method, :foo, :bar ) end end Only need to figure out how to handle the continue or yield. Like you said it might be declarable. However we need to be able to render before and/or after the yield and sometimes instead of. It might be useful to be able to get at the contents of the yield to manipulate it, though I don''t know that we need that for any current directives. We could go with positional render methods something like e.render :before, erb_content( ''text_area'', :obj, :method, :foo, :bar ) e.render :after, ''foo bar'' e.render :nothing # like rails could be an option, however it doesn''t always fit as well since we have needs to render nothing and also render instead of what would normally be rendered. Doesn''t make sense used in context with :before and :after, but maybe that''s ok, use :replace if trying to do something else?? e.render :replace ''foo bar'' # would something like this be used to suppress the output and instead render something else. In the absence of :nothing or :replace then we would copy to destination as planned. If :before, or :after were encountered then their output would preceed or succeed the normal rendering (from dcs.render). If we need to retrieve the dcs.render content for manipulation to we use yield?? That seems to be the new standard in rails views instead of @content_for_layout. Not sure if that would be best or whether that might cause us some issues if we were blocking access to the real yield key word. If we used yield it might be like this localvar = yield # now I can manipulate the contents and output with a render # and since yield was called, we''ll know not to call it again later I also agree with the transition/upgrade strategy, if I port all of our directives over and all the ones that were contributed, I think that will suffice for most users. A quick poll could find out if we need to do more with that. Otherwise I like getting a fresh start at this. The directives will for the most part become dead simple with the new api. Thanks for your input Deb. I want this to be a much simplified api to make directive creation as easy as possible for the 80% of needs out there. I think we are getting close. (now I just need to learn about doing block DSL''s. I will get Jim Weirich''s slides back out, was looking at them the other day. I also looked at Neal Ford''s that you mentioned.) Jeff On 8/1/06, Deb Lewis <djlewis at acm.org> wrote:> Jeff - I like the general direction, not entirely convinced about some of > the specifics yet in the initial proposal. > > I''ll do a little brain-dumping and thinking-through-email here. > > Background context: my mental model of template and directive processing > (and how I''ve been writing the developer document so far) is essentially > that of XSLT - what''s happening is a tree transformation process on a source > DOM tree from the xhtml doc with mv markup into a target rhtml doc with html > and erb markup. The default transformation of an element in the source tree > is to simply copy the element and its attributes to the destination tree. > Directive attribute markup allows for modifying or overriding this default > transformation process. > > A directive is an element attribute with a (string) value containing some > encoding of its processing instructions. It''s instantiated at the start of > the element to which it''s attached, with its (string) attribute value > provided in the constructor; it can perform processing at element start > and/or element end, with access to the source DOM via the dcs arg to > stag/etag methods. Processing may involve modifications to the source DOM > state (say, add/change/delete attribute values), with normal transformation > rendering contining thereafter, or the directive implementation can take > control of the output rendering of its element. > > Two thoughts relative to your arg() service. First, I think I''d like to > generally think of the attribute value along similar lines as an args list > to a ruby method: one or more comma separated values, which are the > positional args, optionally with some trailing set of key => value pairs > that get collected into a hash. With sort of a yaml-ish interpretation, > though, so that you don''t have to be strict about wrapping quotes in the > markup to get a string literal, etc. (?maybe your :quote => true option > could be dropped? superfluous if we can just "do the right thing" for > strings and numbers and then allow the block processing arg from client to > supplement) > > Second, the arg markup done w/in the method implementation seems quite > cryptic to me and somewhat inappropriate - auto-stuffing inst vars may be > overkill, why not return the value and let me decide on temp vs. inst var > w/in my method impl, and actually the declarative-style notation at that > point just seems a bit odd. > > [side remark: i''m a bit skeptical about the current passion to label any and > all form of framework and library protocol design as "a DSL", thereby > attributing Goodness </rant>] > > But if you shift the placement of the arg service into the class declaration > rather than w/in the s/etag processing method then it starts to feel better > to me. Then it *is* a more declarative style notation - it describes the > signature of the directive attribute: I expect some number of positional > args and for each I can state processing options (:quote) or default value > (:default => ''foo''). Then allow last attr_arg to collect a hash - your > :merge option name seems ok for that. Such an attr_arg method > implementation in DirectiveBase takes the declarations and builds a spec > that''s used when the directive is instantiated to process the attribute > value string into useful form, and then it *does* make sense to be popping > the arg values off and putting them in inst vars for the processing > method(s) in the directive implementation to access. > > class MySpecialDirective < DirectiveBase > attr_arg :obj, :quote => true > attr_arg :method, :quote => true > attr_arg :foo, { ''override with this value''} > attr_arg :bar, { |x| x.downcase } # lowercasing the arg before setting it > to @bar > attr_arg :options, :merge => [:common_html, :size, :maxlength] > > def etag(dcs) > ...do some init or munging of the source DOM here... > end > def etag(dcs) > ...output rendering stuff here... > end > > end > > >> To access the full directive attribute value use attr_value > >> To access or set element attributes, use attr[:class] or attr[''class''] > >> To access content of element, use content to get array, or content_str to > get joined version > > I''d say just go with the symbol version of the attr service: attr[:class], > don''t bother with attr[''class''] variation. For accessing element content, > maybe better name is element_content and element_content_str. And > element_content should return the form that you normally want, which I think > might be the string content. > > I need to think a bit more about the rendering methods portions of the > proposal; I still need to wrap my brain around the dcs value that''s provided > as the argument to stag and etag methods in a directive implementation. > What I want when I''m writing a method in a directive implementation is clean > protocol for accessing the source DOM - access or manipulate the element I''m > attached to and its attributes, which I think you''ve almost got now. And > then I want simple operations which specify the effect I want to produce on > the output tree (with default being the copy-to-dest behavior of > SimpleRenderHandler if I do nothing to specify rendering output) > > I''m not sure if it makes sense to break up SimpleRenderHandler and recast it > as a mixin-module (StandardRenderHandler or somesuch) which establishes the > default behavior - include in DirectiveBase and the backstop > SimpleRenderHandler in parser.rb. > > RE: render_nothing - could/should this be done as a declarative statement on > the directive class? > > RE: render continue - think I''d like something closer to the controller/view > style behavior. If the directive doesn''t explicitly invoke a render > operation to specify what it wants to produce in the output, then the > default is to do the standard copy-src-to-dst behavior. > > RE: transition/upgrade strategy - I''m inclined to say this is a good time to > just break directive implementations and "do it right" - get rid of the old > junk, or maybe keep existing guy as as a separate (renamed) transition > upgrade class but *ONLY* if a poll of our existing MV users indicates they > really want and need this. I just don''t think there''s enough of an > installed base in place to muddle the path forward with backwards compat. > If I have directive impls at this early stage in MV''s life cycle, I think > I''d want to get them cleaned up and simplified. So let''s work this out and > send out an advance warning notice on the mailing lists, if there''s major > upset then we can revisit. > > ~ Deb >