Masterview devel - Sep 2006 - Thoughts on directives: attr_arg and related services

Jeff - see below for some thoughts on our DSL notation and directive
implementation framework, now that I''ve been muddling about in the code
and
trying to write documentation to explain the attr_arg DSL notation.

~ Deb

-- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --

:optional_default keyword seems unnecesssarily verbose; why not just
:default => value

   attr_arg :foo, :default => nil

-- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --

:array => :remaining_args notation is reasonably clear, but not
"obvious"
(easy to remember) - doesn''t quite fit with the terminology ordinarily
used
for this when talking about method signatures.  

I think the old C terminology "varargs" is commonly used to talk about
this
kind of thing (e.g., Java 5 picked this up: "...variable arity parameters,
known less formally as varargs").  So maybe :varargs => true

  attr_arg :optional_values_list, :varargs => true

-- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --

I''ve concluded that :merge is a very confusing option name, in two
respects.
First, everything else in the attr_arg domain is about parsing and
manipulating the directive''s attribute value string.  This option is
different - it''s expressing a request to go *outside* the scope of the
directive attribute itself to collect information from its context (other
attributes on the element to which the directive is attached).  

Second confusing aspect is typing of the value - it''s not initially
obvious
that the arg value is always a string literal encoding of a hash, vs.
actually *being* a hash.

Related the latter aspect, as I think about keyword args and options args
which are some combo of keyword-value pairs from the attribute string and/or
sibling attributes from the element itself, I think I''d like an option
to
specify whether I want the hash literal string (default) or an actual hash.
Maybe :hash => true or :eval => :hash or some notation along those lines.

That would let us pull the parsing/evaluation impl out of the Attr directive
into a general directive-helper service and let mv:attr''s impl use an
attr_arg decl indicating that it wants its arg eval''d into a real hash.

Possibly even something more general, e.g., :eval => true or :type =>
:hash
(or :array or :int, depending on how far we want to go with this)

We do need to make sure we have good exception-handling code so that typos
and mistakes in template markup fail politely!  (haven''t pushed on that
area
much, but the one time I tripped over this it was quite confusing)

-- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --

Accessor names in DirectiveBase: would like some of these names to be
clearer in distinguishing operations on the directive itself (its args,
processing state, rendering state) vs. operations on the element or the
overall template processing.

Specifically: attrs and tag_name would be much clearer if named, say,
element_attrs and element_tag

-- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --

On 9/18/06, Deb Lewis <djlewis at acm.org> wrote:
> Jeff - see below for some thoughts on our DSL notation and directive
> implementation framework, now that I''ve been muddling about in the
code and
> trying to write documentation to explain the attr_arg DSL notation.
>
> :optional_default keyword seems unnecesssarily verbose; why not just
> :default => value
>
>    attr_arg :foo, :default => nil
Yeah, that seems like a sensible change. I named it as I did since it
was only getting invoked if it was needed, meaning that it wasn''t
getting called unless something later in the parameter stack changed
to trigger it. (an optional param, that if needed would default to x).
  So for instance lets say you didn''t have any html options in your
form helper but because MasterView merges in your class style or
something into it, now you need a default for the options arg, thus
the optional_default.

Shortening to :default is probably fine unless that implies that it
will always result in something happening (default being set
regardless). I was just trying to keep the generated form helpers as
clean as what we would do by hand, not setting a bunch of extra values
if they aren''t needed. (actually we might even be able to simply
further when you get down to reading response to :type suggestion).
>
> -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>
> :array => :remaining_args notation is reasonably clear, but not
"obvious"
> (easy to remember) - doesn''t quite fit with the terminology
ordinarily used
> for this when talking about method signatures.
>
> I think the old C terminology "varargs" is commonly used to talk
about this
> kind of thing (e.g., Java 5 picked this up: "...variable arity
parameters,
> known less formally as varargs").  So maybe :varargs => true
>
>   attr_arg :optional_values_list, :varargs => true
I agree :varargs => true is probably clearer than the :array,
:remaining_args terminology. I was thinking about the notation
allowing other types of arrays to be created, but I don''t think that
we had any business case for any of these yet so I think :varargs
would be simpler and clearer.
>
> -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>
> I''ve concluded that :merge is a very confusing option name, in two
respects.
> First, everything else in the attr_arg domain is about parsing and
> manipulating the directive''s attribute value string.  This option
is
> different - it''s expressing a request to go *outside* the scope of
the
> directive attribute itself to collect information from its context (other
> attributes on the element to which the directive is attached).
>
Right, that is a different aspect of this. A commonly used aspect in
the MasterView world but something that might not be as clear. I was
basing it off hte Hash.merge method in Ruby, but if you can propose a
better key word then that''s fine.
> Second confusing aspect is typing of the value - it''s not
initially obvious
> that the arg value is always a string literal encoding of a hash, vs.
> actually *being* a hash.
Right, that is a tricky area since it is a string representation of a
hash which allows the shorthand method argument style declaration
(omitting the brackets).
>
> Related the latter aspect, as I think about keyword args and options args
> which are some combo of keyword-value pairs from the attribute string
and/or
> sibling attributes from the element itself, I think I''d like an
option to
> specify whether I want the hash literal string (default) or an actual hash.
> Maybe :hash => true or :eval => :hash or some notation along those
lines.
I haven''t come across the need for an actual hash yet, though it had
crossed my mind. I originally started out using evals to create
hashes, but found that confusing and tricky for users, such that I
abandoned the idea (or at least my implementation). If you see this
being a need then we can probably fit it in to the definition.
>
> That would let us pull the parsing/evaluation impl out of the Attr
directive
> into a general directive-helper service and let mv:attr''s impl use
an
> attr_arg decl indicating that it wants its arg eval''d into a real
hash.
>
> Possibly even something more general, e.g., :eval => true or :type =>
:hash
> (or :array or :int, depending on how far we want to go with this)
If we indicate that it is hash literal string, then we can infer what
it''s default value would be {} getting rid of the :optional_default.
Same for an array we could infer that it is []. I don''t remember if we
needed :optional_default for any other types of defaults (boolean,
int, string) but there are reasonable defaults for those too (false,
0, '''' or nil).

I had originally wanted to define types of params but then kind of
went away from that for simplicity. Also it didn''t add anything to
parsing since I couldn''t really use the fact tht something was a
number anyway (since it might be a method that returns a number), ...
Maybe it is clearer to define the type especially if you want some
default capability. So rather than defining the optional_default, you
define the type and the default will occur automatically.

Maybe something like this?? If you want to define the type :type =>
:hash  or :type => :array then the system will automatically create a
default of {} or [] if one is needed when passing in parameters to a
method call. Otherwise specifying the type is optional. If nothing is
specified and param is needed it will default to nil.

So maybe to summarize the above points we can do something like

attr_arg :foo, :type => :hash # hash literal string with default of {}
if needed for param
attr_arg :foo, :type => :array # array literal string with default of
[] if needed for param
attr_arg :foo, :varargs => true # array or remaining args
attr_arg :foo, :type => :hash, :merge => [:bar, :baz]

Is this clearer?? We can certainly add the eval => true or something
if needed or if you can show me how this might be used so we can write
some tests around it. Visualizing all of this is half the problem. :-)

>
> We do need to make sure we have good exception-handling code so that typos
> and mistakes in template markup fail politely!  (haven''t pushed on
that area
> much, but the one time I tripped over this it was quite confusing)
Yeah that is an art in itself. I think we can do better in this area
as well. I only have started to touch on this.
>
> -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>
> Accessor names in DirectiveBase: would like some of these names to be
> clearer in distinguishing operations on the directive itself (its args,
> processing state, rendering state) vs. operations on the element or the
> overall template processing.
>
> Specifically: attrs and tag_name would be much clearer if named, say,
> element_attrs and element_tag
>
> -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
I see your point. element_attrs and element_tag is fine with me if you
think it is clearer. Feel free to suggest any other specific changes
that you think make more sense. I was going for shorter, however
sometimes that is not always the best, especially if easily confused
with something else.

Thanks for your input. Let me know what you think about my comments
and I can make the changes. I am talking about DSL''s and this next
Tuesday so I would like to make any clarifying changes this week (as
long as they are mostly syntactical and not too much in forms of
changing functionality), so that it will be clearer to my audience as
well.

I appreciate your input! I got my new server configured and setup with
some decent security measures, so I can get back into looking at the
new documents you mentioned last week.

Jeff

p.s. Aside from essentially minor quibbling about some names and such, I
really really like everything you did with this rework.  The declarative
notation to express a directive signature and its parsing attributes into
argument values, along with the event handler mechanism to configure the
processing logic are way cool.

~ Deb

ok, time for another round after contemplating your responses and some of
the factors you''ve already considered.

Let''s stay with your current concept that attr_arg simply produces
string
values and not introduce typing.  As you say, it''s not yet clear
it''s needed
and better to not introduce if it only complicates things.

So attr_arg has a clear point of view: it operates on a string (the
attribute_value string) and produces strings.  The only time you get
something other than a string from an attr_arg declaration is for the
:remaining_args (:varargs?) case, when you get an array of string values.

[of course you can also provide a block and transform the string before it
gets assigned to the arg inst var, but then that''s yer own fault, er,
responsibility]

Provide helper function in DirectiveHelpers which parses a string value
which is a hash literal (with or w/out the {} to handle syntactic sugar
shorthand) - call it parse_string_into_hash or something like that.
(extract from the mv:attr impl)  Then if a directive handler really wants an
actual hash out of its attribute value or an arg, it has a standard way to
easily do so.

?Maybe also a serialize_hash_to_attributes helper?  consider if we have use
case to justify, easy to add

I do think the :merge option needs a better name.  Merge is a collection
operation on hashes and sets, while the subject of an attr_arg is a string
(even though a string which happens in this case to be a hash literal).
Calling it :merge implies it''s operating on a hash, which
isn''t the case.
Also, what :merge actually does is go outside the scope of *this* directive
and its attr_value to collect additional values from the directive''s
context, so I''d like a name which somehow expresses "going out and
collecting additional attr name/value pairs to include in whatever we parsed
out of the directive''s own attribute value"

:append_element_attrs perhaps?!

RE: :optional_default vs :default - don''t want to change the existing
semantics, the shorter name just seemed sufficiently clear to me.

RE: DirectiveBase accessors for element attr''s and tag name vs. the
directive''s own attr_value
>> element_attrs and element_tag is fine with me if you think it is
clearer.

attrs and tag_name on DirectiveBase still feel wrong/confusing to me as I
continue to work with this stuff, so I still like some sort of change in
those accessor names.  Within a directive implementation, my focus is this
directive and its value.  The directive itself *is* an attribute; the
element that it belongs to *has* attrs and a tag name, but that''s about
the
object that it belongs to and not the directive itself.

Every time I run into an attrs reference in a directive I find I have to
stop and think for a moment and that''s usually an indication of a
naming
problem.

~ Deb

On 9/20/06, Deb Lewis <djlewis at acm.org> wrote:
>
> So attr_arg has a clear point of view: it operates on a string (the
> attribute_value string) and produces strings.  The only time you get
> something other than a string from an attr_arg declaration is for the
> :remaining_args (:varargs?) case, when you get an array of string values.
I think I like your idea of changing :array => :remaining_args
to simply :varargs => true
>
> Provide helper function in DirectiveHelpers which parses a string value
> which is a hash literal (with or w/out the {} to handle syntactic sugar
> shorthand) - call it parse_string_into_hash or something like that.
> (extract from the mv:attr impl)  Then if a directive handler really wants
an
> actual hash out of its attribute value or an arg, it has a standard way to
> easily do so.
Lets put this on the todo list, don''t need it right now, but would be
good to have for developers so done in consistent way.
>
> ?Maybe also a serialize_hash_to_attributes helper?  consider if we have use
> case to justify, easy to add
If I understand what you are saying here is to have a helper that
takes all the values in a hash and sets the attributes on the element
from them. Similar to how the attr directive works. We can probably
take the code from attr directive and put in a helper and just have
attr call the helper.
>
> I do think the :merge option needs a better name.  Merge is a collection
> ...
> :append_element_attrs perhaps?!
ok, I think for clarity you have a good point

:merge will become :append_element_attrs
>
> RE: :optional_default vs :default - don''t want to change the
existing
> semantics, the shorter name just seemed sufficiently clear to me.
will go with :default vs :optional_default
>
> RE: DirectiveBase accessors for element attr''s and tag name vs.
the
> directive''s own attr_value
>
attrs will become element_attrs
tag_name will become element_tag

------------------------

So to summarize the changes I will make:

:array => :remaining_args  becomes  :varargs => true
:merge will become :append_element_attrs
:optional_default will become :default
attrs will become element_attrs
tag_name will become element_tag

For todo list

DirectiveHelpers.parse_string_into_hash
DirectiveHelpers.serialize_hash_to_attributes # refactor attr
directive code into here

Let me know if I have forgotten anything. Thanks for the input! The
more refinement we put in now, the less questions and confusion we
will have down the road :-)

Jeff

On 9/18/06, Deb Lewis <djlewis at acm.org> wrote:
> p.s. Aside from essentially minor quibbling about some names and such, I
> really really like everything you did with this rework.  The declarative
> notation to express a directive signature and its parsing attributes into
> argument values, along with the event handler mechanism to configure the
> processing logic are way cool.
>
Thanks, I do like how it came out. I''m glad that you do provide your
input on the naming and suggestions for improvement though, naming is
not my strong suit :-) and the better job we do at this now, will save
us tons later (in questions and confusion).

I am looking forward to talking about it all next Tuesday at the St.
Louis Ruby meeting. When is your OC meeting?

Jeff

Jeff - good, I like it.  :append_element_attrs is certainly more verbose,
but clear

Do you want me to commit my changes which made DirectiveDSL into a mixin for
DirectiveBase and added some internal namespacing on its impl mechanisms
before you do this?  I haven''t made any other changes, just this
structural
stuff, but didn''t want to submit unless you concurred with these
adjustments.

~ Deb

------------------------

So to summarize the changes I will make:

:array => :remaining_args  becomes  :varargs => true 

:merge will become :append_element_attrs 

:optional_default will become :default 

attrs will become element_attrs, tag_name will become element_tag

------------------------

>> I am looking forward to talking about it all next Tuesday 
>> at the St. Louis Ruby meeting. When is your OC meeting?
Supposed to be next Thu, tho we''ve lost our prev. mtg location and are
looking for a new home

~ Deb