On 2011-01-15, at 06:28, André Bargull wrote:

> On 1/15/2011 12:32 AM, P T Withington wrote:
>> On 2011-01-14, at 18:15, André Bargull wrote:
>>> Concerning your statement in JIRA:
>>>> But if I can generate a more correct schema without relying on the 
>>>> developer getting the documentation just right, I would rather do that.
>>> 
>>> The schema pretty relies on doc comments, especially the @lzxtype is really 
>>> important - not only for identifying events.
>> 
>> In theory, a developer is allowed to add an instance-var to a built-in class 
>> (say `foo`), and they are not required to declare a corresponding event (or 
>> define a setter).  How do we generate a correct schema, given that the 
>> current semantics say I am allowed to listen for `onfoo`, even if the 
>> developer did not declare it?
>> 
> 
> It's possible to listen to any event in a <handler>, regardless whether it 
> exists or not. <handler name="ontext"> works just as <handler 
> name="ontextorwhateverthisiscalled">. But the expanded form with <handler> 
> isn't important for this bug, we only need to consider the short form. So the 
> bug report states that the short form ontext="{code}" does no longer work, 
> because the <event name="ontext"> entry is missing in the lfc.lzx schema 
> file. And we also only need to consider lfc lzs-classes to lfc.lzx schema 
> file.
> 
> Why do I vote against adding an implicit event for every var declaration? 
> Simply because there isn't a corresponding event sent for every var 
> declaration (examples: onparent, onimmediateparent, onsubnodes, onsubviews, 
> etc.).

Of course, those are technically bugs.  The rule is supposed to be that if you 
define a setter, you send the event.  This date backs to the original OL design 
of setters, where one of the designers felt it was really important to have 
control over the event behaviour in custom setters.  As you notice in LPP-3002 
I suggest that the default should be to send an event and that you have to do 
something special in your setter definition to say you want to override the 
default...

People are always forgetting this rule.  Especially in the LFC.

When they do remember, they have to declare the event, and because our doc 
tools default to documenting things unless you explicitly mark them as private 
(another default I think is in the wrong direction), the doc gets cluttered 
with these unnecessary events (because they are implicit).

You are also right that historically we have only promoted certain events to be 
assignable in the open tag (the ones that make it into the schema).

The root of this bug is that I took a declared event an marked it private 
(because it was cluttering the doc) which also removed it from the schema, 
because I did not know the pattern you supplied to mark it private but declare 
it as an event so that it _is_ in the schema and is _not_ in the doc.

Knowing that, I agree with your review and will back out my over-agressive 
schema builder change and put in the declaration you suggest.

Left open for future generations to ponder:

  1) what are the citeria for declaring and event or not?
  2) can the schema generator warn you when it discovers a setter and no 
corresponding event?

> And this might give users the false impression that the event actually exists 
> and is also fired at runtime. So when she writes "onparent={code}", but that 
> code is never executed, it's a bit confusing. The compiler should have warned 
> that "onparent" does not exist. <handler name="onparent"> won't fire, too, 
> but as explained earlier <handler> is a different story [1]. In addition to 
> that, we've also moved away from the implicit event thing for the lfc 
> classes. So back in the days we've used this pattern for firing events:
>> if (this['on' + attr]) this['on' + attr].sendEvent(value);
> 
> But now all events are declared explicitly in the lfc. So the additional 
> check whether the event exists is no longer necessary.

But not in user code.  See the definition of setAttribute, which looks for 
implicit (or added at runtime) events.

> In conclusion we can rely that events are declared explicitly in the lfc, but 
> maybe the developer missed to add the required "@lzxtype event" doc-comment. 
> We could extend the schema builder to warn for this case:
>> /** @access public */
>> var foo;
>> /** @access private */
>> var onfoo = LzDeclaredEvent;
> 
> Here the developer added a public attribute `foo` and declared the 
> corresponding event `onfoo`, but missed to add "@lzxtype event" for the 
> event. So the rule would be:
> If a var declaration `zot` exists and the corresponding event `onzot`, but 
> `onzot` does not include "@lzxtype event", emit a warning when building the 
> schema.
> 
> Open question: How to proceed for events without var declarations?
>> /** @access public */
>> var onbar = LzDeclaredEvent;
> 
> We'd like to emit a warning for this case, too. Looking at the rhs of the 
> assignment might give a clue that this an event, but "LzDeclaredEvent" only 
> works for plain, old events, extended events like the scroll events in <text> 
> are more difficult to catch. Maybe it's possible to always emit a warning for 
> any public declaration if the @lzxtype doc-comment is missing?

Please file an improvement suggestion for this.

> [1] We should consider removing this discrepancy between <handler> and the 
> short form for event listeners. That means only allowing <handler> if there 
> is a corresponding <event> declared explicitly (resp. implicitly for lzx 
> <attribute>s). Whether that event is actually fired at runtime is out of 
> scope for us. But see Sarah's improvement request LPP-3002.

I'm not sure we can do that, since technically the language allows you to 
dynamically add events at runtime.  I'm sure someone's app will fail if we stop 
supporting that.


Reply via email to