Skip to content

Fix template for parameter types in callbacks #50

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

esx
Copy link

@esx esx commented Feb 13, 2014

Support for multiple types (like 'Number or String') in callback
signatures. See for example signatures in documentation for jQuery.map.

Support for multiple types (like 'Number or String') in callback
signatures. Se for example signatures in documentation for jQuery.map.
@scottgonzalez
Copy link
Member

jQuery.map() has an incorrect signature. arrayOrObject is not a valid parameter for the signature. That signature should only be documenting objects.

@esx
Copy link
Author

esx commented Feb 17, 2014

jQuery.map() was a bad example since there was an error in how the signature was described. ".prop( propertyName, function(index, oldPropertyValue) )" is a better example, since the callback can receive multiple different types depending on the property requested. So we need to be able to show "union"-types in callback signatures, just as with ordinary parameters and return values.

@scottgonzalez
Copy link
Member

I don't see how listing every single data type provides any benefit. We should use a meta type like Mixed or Any.

@esx
Copy link
Author

esx commented Feb 17, 2014

Do you mean in the general case, or just in the case of "prop()"? I think I agree with you in the case of "prop()", since properties can be many different types (e.g. "ownerDocument" is a Document for example, so at least four different types.). But this is really an issue regarding the documentation of specific methods. This commit is just a general xslt fix so that multi-types is rendered the same way in callback signatures as in other parameters.

@scottgonzalez
Copy link
Member

I refuse to land a change without a specific example of where the functionality is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants