Skip to content
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

Weird data-lt attribute for IDL methods with one optional param? #3200

Closed
tidoust opened this issue Nov 6, 2020 · 4 comments
Closed

Weird data-lt attribute for IDL methods with one optional param? #3200

tidoust opened this issue Nov 6, 2020 · 4 comments
Labels

Comments

@tidoust
Copy link
Contributor

tidoust commented Nov 6, 2020

Important info

Description of problem

The generated data-lt attribute contains a leading comma that looks weird. For instance, the attribute's value is complete()|complete(, result) for the PaymentResponse.complete method. I would rather expect complete(, result) to be complete(result).

That seems to only happen when there is only one argument that is optional.

@tidoust
Copy link
Contributor Author

tidoust commented Nov 6, 2020

Digging into the code, I suspect that this happens due to:
https://github.com/w3c/respec/blob/develop/src/core/dfn-finder.js#L100

const result = `${operationName}(${requiredArgs}${
  optionalArgs ? `, ${optionalArgs}` : ""
})`;

... could perhaps be replaced by:

const result = `${operationName}(${
  requiredArgs && optionalArgs ? `${requiredArgs}, ${optionalArgs}` :
  requiredArgs ? requiredArgs :
  optionalArgs ? optionalArgs : ""
})`;

@marcoscaceres
Copy link
Contributor

Option 1 would be my preference.

tidoust referenced this issue in w3c/reffy Nov 12, 2020
- Ignore constructor definitions for HTML elements, the spec handles them
separately
- Ignore specs that don't respect the dfns data model such as WebGL
- Work around ReSpec's issue https://github.com/w3c/respec/issues/3200

The update also improves the reporting's format with expandable sections, and
a clearer message when a possible candidate definition has been found.
tidoust added a commit to tidoust/respec that referenced this issue Nov 13, 2020
This update fixes the generation of the `data-lt` attribute when an operation
only has optional arguments. The code added a `, ` prefix before the list of
optional arguments, regardless of whether the operation had mandatory arguments
as well, leading to names such as `methodName(, options)` instead of
`methodName(options)` (see issue speced#3200).
@tidoust
Copy link
Contributor Author

tidoust commented Nov 13, 2020

Yes, I love option 1 too, all the more since I did not suggest any option 2 ;)
I prepared PR #3208 accordingly.

tidoust referenced this issue in w3c/reffy Nov 16, 2020
The definitions checker compares CSS, dfns, and IDL extracts created by Reffy
to detect CSS/IDL terms that do not have a corresponding dfn in the spec.

The checker outputs Markdown that is typically suitable for inclusion in a
GitHub issue. It may also return the result as JSON.

Some specific scenarios are being handled to avoid false positives:
- Ignore constructor definitions for HTML elements, the spec handles them
separately
- Ignore specs that don't respect the dfns data model such as WebGL
- Work around ReSpec's issue https://github.com/w3c/respec/issues/3200
- Ignore special getter, setter, deleter methods
- Handle overloaded methods (imperfectly)

Also the checker reports warnings when it could find a definitions that is
correct but not "perfect":
- when a method definition exists but parameter names are different, e.g.
`setCustomValidity(message)` instead of `setCustomValidity(error)`.
- when method definition does not include the parameters, e.g. `add()`.
- when a method definition exists without parentheses, e.g. `drawImage`.
- when a single method definition exists for overloaded methods, e.g.
`setRangeText` or `drawFocusIfNeeded`.
@sidvishnoi
Copy link
Member

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

No branches or pull requests

3 participants