.on() should document that handler accepts .trigger() args

[dmethvin]

See DefinitelyTyped/DefinitelyTyped#1916

[mattlunn]

I'm trying to assist on this. Does anyone (@dmethvin?) have any suggestions on how to represent these optional additional parameters in the signature?

If we have the signature defined like so:

<argument name="handler" type="Function">
  <desc>A function to execute when the event is triggered. The value <code>false</code> is also allowed as a shorthand for a function that simply does <code>return false</code>.
  <argument name="eventObject" type="Event" />
  <argument name="extraParameter" type="Anything" optional="true" />
</argument>

... then I forsee the following problems:

1. optional="true" does not actually add any styling to the generated page. Am I doing this correctly?
2. It doesn't signify the fact extraParameter can be provided multiple times.

To fix 2, I considered:

<argument name="handler" type="Function">
  <desc>A function to execute when the event is triggered. The value <code>false</code> is also allowed as a shorthand for a function that simply does <code>return false</code>.
  <argument name="eventObject" type="Event" />
  <argument name="extraParameters" type="Anything" optional="true" />
  <argument name="extraParameters" type="Anything" optional="true" />
</argument>

... based on past usage in deferred.always(), however the signature still lacks the "optional" indication.

[dmethvin]

@kswedberg You're my guru on the XML for specifying this. Any ideas?

[scottgonzalez]

optional="true" does not actually add any styling to the generated page. Am I doing this correctly?

Well, you don't really want to mark the arguments to a function as optional, do you? We're not documenting optional parameters for a user at this point. We can certainly make this work, but I'm not sure we want to. Here's what I have with local edits to grunt-jquery-content based on your markup:

It doesn't signify the fact extraParameter can be provided multiple times.

We can continue to do what we're doing with deferred.always() or we can try to add support for varargs in grunt-jquery-content.

[mattlunn]

You don't really want to mark the arguments to a function as optional, do you?

Mmmm, I sort-of think I do. Maybe the word optional is a bit of a misnomer... but I can't think of a better word that represents only-there-if-the-user-passes-arguments-to-trigger.

The mockup you provided is a clean way (in my eyes) to signify that eventObject is always provided, and extraParameter isn't. There would then be a section in the docs which explains when extraParameter is provided.

How else would you propose we show this? Would you prefer to leave the signature as-is, and then just document the extraParameter behaviour in the docs?

We can continue to do what we're doing with deferred.always() or we can try to add support for varargs in grunt-jquery-content.

I think how it's presented in deferred.always() is fine TBH; no need for additional display options.

[scottgonzalez]

I guess if the TypeScript docs use this, then we probably should too. But that would imply we should also support varargs. I've filed issues for both of these. I'll handle the optional arguments now since that part is easy.

[scottgonzalez]

I've added support for optional arguments in callbacks, released grunt-jquery-content 0.11.2, and updated this repo to use the new version. If you pull the update and run npm install, your signature should work (you're missing the closing </desc> though).

As for varargs, it turns out I added this a long time ago, but we're only using it in one place (and it's not supported in callbacks right now). You can see it in use at http://api.jqueryui.com/jQuery.widget/#method-_super, but I'm not sure if that's the way we want to represent it.

[mattlunn]

Ok. I tried using the approach we use for deferred.always() to represent the varargs nature of extraParameter, but it looked crap... it doesn't really signify that multiple extraParameter's can be provided.

However, it's possible to hack in vararg syntax, via:

<argument name="..." optional="true" />

... which renders as:

This, IMO, looks better, but I'm not sure whether "looks better" can be universally agreed, or whether hacking in vararg support would be recommended. @scottgonzalez or @dmethvin , any thoughts?

FWIW, I've already pushed my changes for enabling the hacked-in-vararg support to https://github.com/mattlunn/api.jquery.com/tree/472, and it's ready to PR if given the 👍

[scottgonzalez]

I've already pointed out that varargs support exists. See #472 (comment)

[mattlunn]

@scottgonzalez: I Interpreted your statement:

... it's not supported in callbacks right now

... as it wouldn't be supported in this context. Indeed, when I use the syntax:

<argument name="extraParameter" type="Anything" optional="true" rest="true" />

I just see extraParameter being defined as a normal, optional parameter, with no visual indicator that it is "many".

Saying that though, no exception is thrown when the XML is parsed. Are you saying you want me to make the PR using rest="true", so that when support for varargs in callbacks is added, it'll "just work"? It would mean that, in the meantime, extraParameter looks like any-old-parameter.

[scottgonzalez]

Yes, it won't work right now. But we shouldn't hack in support for it, we should do it properly.

[mattlunn]

Understood. Code updated and PR submitted using role=true.