Re: [heycam/webidl] Allow extended attributes to apply to types (#286)

I'm pretty swamped with reviews for various stuff right now, so it will likely take me a few days to get to this.  I'm sorry.  I simply have a huge backlog, both in terms of specs and in terms of code.  :(

You should probably look at https://lists.w3.org/Archives/Public/public-script-coord/2017JanMar/0009.html in case it affects your approach here.

One question I do have up front: what is the behavior you're actually aiming for with the patches applied?  Knowing that will let me evaluate the behavior, as well as whether the actual spec changes implement that behavior correctly.  Ideally, that sort of thing would live in the commit message...  I ask this because you say you're implementing the plan discussed in https://lists.w3.org/Archives/Public/public-script-coord/2017JanMar/0003.html but I actually outlined a few different options in that mail, and I'm not sure which ones you decided to take.

Some responses to questions without having read the changes very carefully yet.

> Is [Clamp] long being a distinct type from long a good way of doing this? 

That seems reasonable, at first glance, yes.

Will take some thought to check whether expressing interactions like "not both [Clamp] and [EnforceRange]" works right in that setup.

> Is it sufficiently clear that extended attributes propagate through typedefs?

I don't think it is.  In particular, it's not obvious to me _how_ they propagate, exactly, based on searching the diff for "typedef".  Like what the actual result is.  I don't quite recall what model the spec has of typedefs in general, nor what implementations do, exactly...  :(

> We have separate TypeWithAttributes and Type

That's fine, I think.   ReturnType definitely shouldn't have attributes.  Then again, ReturnType is already separate from Type, so we don't really care about this part.

There are other places where types probably shouldn't have attributes (at least not the attributes we're defining).  For example, callback argument types, right?

> I didn't allow (gramatically) specifying an extended attribute over an entire union.

I'm not sure what you mean.  Your grammar has "ExtendedAttributeList UnionType Null" as a possible thing for TypeWithAttributes, right?

> This will invalidate a lot of existing IDL which will need to get updated.

Hmm.  So we _could_ support the old syntax, in theory, for attributes.  As you note, it would be complicated.

In practice, TreatNullAs is ony used in a few specs: HTML, CSSOM, DOM.  Updating them would not be a huge deal.  I see ~40 places in Gecko's IDL using this annotation, which is not so bad to just update en-masse.

> Can we nuke that part of the spec?

I think so.  I see no consumers in Gecko either, and we really shouldn't add new ones, imo.

> can we just get rid of the silly identifier argument

That's probably fine, yes.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/heycam/webidl/pull/286#issuecomment-275307976