-
Notifications
You must be signed in to change notification settings - Fork 20.6k
Do you guys support signed string hex for jQuery.isNumeric #2781
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
Comments
In the first case, the JS parser converts the hex before execution. In the second, you're expecting the string to be converted at the point of execution. Hex constants aren't supported by Edit: |
Thanks @dmethvin , looks like this needs |
I think this can be fixed in docs. Negative hex constants aren't a really common use case. |
Just note: Also I think I think it could be its harder to understand it now as the useful comments are removed in #2663. |
With respect to the initial question, |
@gibson042 I'm fine if |
The purpose of Personally, I would be comfortable with an implementation like ¹ e.g., |
I've looked at the docs and it seems very vague and even incorrect to me. The short description states:
which is simply not true as that would imply the argument has to be of a number type. Later in the text it states:
without explaining what "a numeric value" is. Therefore, I can see why no one really knows what this method tries to do. We should determine what's the goal of this function and document it properly, then we can fix the edge cases. |
I agree with that @mzgol, it's really something like, "Determines whether the argument (typically a String) can be converted to a valid Number. If the argument is already a Number, returns true unless it is one of the exceptional values like Instead of trying to chase edge cases we can just add some clarifying examples to the docs page, these were taken right out of the unit tests. If someone needs input validation beyond that they should implement it themselves. This is another case where we waded into a huge amount of 💩 for a seemingly trivial utility function that has nothing to do with the DOM! |
Keep in mind we've already added a note to |
@timmywil Good point but my remarks still stand. A note is just a note, kind of additional info whereas I think the main portion of the page almost doesn't contain any concrete information... |
Since we no longer have any internal uses of In its current state, I'd characterize the function's behavior as:
If everyone agrees, then I suppose an api.jquery.com PR is in order. |
I think that description is fine @gibson042 and would just add another example showing that |
I feel its better to remove this. Some reasons are already mentioned above and I wanna add a few more:
|
Opened docs issue. jquery/api.jquery.com#862 |
Add back accidentally deleted comments about the implementation. Add tests for new number literals. Ref jquery#2663 Ref jquery#2781 Ref jquery#2780
Add back accidentally deleted comments about the implementation. Add tests for new number literals. Ref jquery#2663 Ref jquery#2781 Ref jquery#2780
Is it the right behaviour?
The text was updated successfully, but these errors were encountered: