Re: [heycam/webidl] Link to ES for "array index" (#427)

tobie commented on this pull request.

Nice changes moving away from the hacky original solution. A couple of comments inline for discussion. LMK what you think.

> @@ -12188,18 +12190,19 @@ internal method as follows.
 
 <h4 id="legacy-platform-object-abstract-ops">Abstract operations</h4>
 
-<div algorithm>
+<div algorithm="to determine if a property name is an array index">

You don't need a value for the "algorithm" attribute if there's a DFN in it. (Ideally, all algorithm would have a dfn and wouldn't require anything but a boolean "algorithm" attribute.

>  
-    The name of each property that appears to exist due to an object supporting indexed properties
-    is an <dfn id="dfn-array-index-property-name" export>array index property name</dfn>,
-    which is a property name |P| for which the following algorithm returns true:
+    To determine if a property name |P| <dfn lt="is an array index|is not an array index"
+    oldids="dfn-array-index-property-name">is an [=array index=]</dfn>, the following algorithm is
+    applied:
+
+    1.  If [=Type=](|P|) is not String, then return <emu-val>false</emu-val>.
+    1.  Let |index| be [=!=] <a abstract-op>CanonicalNumericIndexString</a>(|P|).
+    1.  If |index| is <emu-val>undefined</emu-val>, then return <emu-val>false</emu-val>.
+    1.  If |index| is less than 0 or is greater than or equal to 2<sup>32</sup> − 1, then return

To properly close #409, I think you want to put a small note right here about what this number represents (e.g. it's 1 less than the maximum array length as defined in ES).

>  
-    The name of each property that appears to exist due to an object supporting indexed properties
-    is an <dfn id="dfn-array-index-property-name" export>array index property name</dfn>,
-    which is a property name |P| for which the following algorithm returns true:
+    To determine if a property name |P| <dfn lt="is an array index|is not an array index"
+    oldids="dfn-array-index-property-name">is an [=array index=]</dfn>, the following algorithm is

I'm not too sure I like the name change. Aren't array indices numbers? And aren't we here verifying they're in fact a string? While more verbose, the old name made that a lot more obvious.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/heycam/webidl/pull/427#pullrequestreview-58582254