Re: [heycam/webidl] Add the ability to construct a callback function (#328)

tobie approved this pull request.

I like the WebIDL arg refactoring. That's pretty neat.

We should indeed clean-up the completion story. I'm wondering whether that's something for WebIDL to define more precisely, or for the infra standard flow control.

> @@ -12166,14 +12166,37 @@ a [=callback interface=] that:
 *   has one or more [=regular operations=] that all have the same [=identifier=],
     and no others.
 
+A <dfn>Web IDL arguments list</dfn> is a [=list=] of values each of which is either an IDL value or
+the special value “missing”, which represents a missing optional argument.
+
+<div algorithm="convert an IDL arguments list to an ECMAScript arguments list">

Since you have a dfn within the algorithm, you don't need to give the `algorithm` attribute a value.

> +    ECMAScript arguments list</dfn>, given a [=Web IDL arguments list=] |args|, perform the
+    following steps:
+
+    1.  Let |esArgs| be an empty [=list=].
+    1.  Let |i| be 0.
+    1.  Let |count| be 0.
+    1.  While |i| &lt; |args|'s [=list/size=]:
+        1.  If |args|[|i|] is the special value “missing”, then [=list/append=]
+            <emu-val>undefined</emu-val> to |esArgs|.
+        1.  Otherwise, |args|[|i|] is an IDL value:
+            1.  Let |convertResult| be the result of [=converted to an ECMAScript value|converting=]
+                |args|[|i|] to an ECMAScript value. Rethrow any exceptions.
+            1.  [=list/Append=] |convertResult| to |esArgs|.
+            1.  Set |count| to |i| + 1.
+        1.  Set |i| to |i| + 1.
+    1.  [=list/Remove=] all items from |esArgs| except the first |count|, so that |esArgs|'s

"Truncate" is a lot clearer to read, even though it's not defined in infra. Should it be defined there? (see https://github.com/whatwg/infra/issues/100)

> +    To <dfn lt="converting" for="Web IDL arguments list">convert a Web IDL arguments list to an
+    ECMAScript arguments list</dfn>, given a [=Web IDL arguments list=] |args|, perform the
+    following steps:
+
+    1.  Let |esArgs| be an empty [=list=].
+    1.  Let |i| be 0.
+    1.  Let |count| be 0.
+    1.  While |i| &lt; |args|'s [=list/size=]:
+        1.  If |args|[|i|] is the special value “missing”, then [=list/append=]
+            <emu-val>undefined</emu-val> to |esArgs|.
+        1.  Otherwise, |args|[|i|] is an IDL value:
+            1.  Let |convertResult| be the result of [=converted to an ECMAScript value|converting=]
+                |args|[|i|] to an ECMAScript value. Rethrow any exceptions.
+            1.  [=list/Append=] |convertResult| to |esArgs|.
+            1.  Set |count| to |i| + 1.
+        1.  Set |i| to |i| + 1.

Increment? (see whatwg/infra#99).

> +    1.  Let |completion| be an uninitialized variable.
+    1.  Let |F| be the ECMAScript object corresponding to |callable|.
+    1.  If [=!=] [=IsConstructor=](|F|) is <emu-val>false</emu-val>, throw a
+        <emu-val>TypeError</emu-val> exception.
+    1.  Let |realm| be |F|'s [=associated Realm=].
+    1.  Let |relevant settings| be |realm|'s [=Realm/settings object=].
+    1.  Let |stored settings| be |callable|'s [=callback context=].
+    1.  [=Prepare to run script=] with |relevant settings|.
+    1.  [=Prepare to run a callback=] with |stored settings|.
+    1.  Let |esArgs| be the result of [=Web IDL arguments list/converting=] |args| to an ECMAScript
+        arguments list. If this throws an exception, set |completion| to the completion value
+        representing the thrown exception and jump to the step labeled
+        <a href="#construct-return"><i>return</i></a>.
+    1.  Let |callResult| be [=Construct=](|F|, |esArgs|).
+    1.  If |callResult| is an abrupt completion, set |completion| to
+        |callResult| and jump to the step labeled <a href="#construct-return"><i>return</i></a>.

I know we use steps labelled "return" all over the place. I still find that icky. Of course we shouldn't change it here.

> +                href="#construct-return"><i>return</i></a>.
+            1.  Append |convertResult|.\[[Value]] to |esArgs|.
+            1.  Set |count| to |i| + 1.
+        1.  Set |i| to |i| + 1.
+    1.  Truncate |esArgs| to have length |count|.
+    1.  Let |callResult| be [=Construct=](|F|, |esArgs|).
+    1.  If |callResult| is an abrupt completion, set |completion| to
+        |callResult| and jump to the step labeled <a href="#construct-return"><i>return</i></a>.
+    1.  Set |completion| to the result of [=converted to an IDL value|converting=]
+        |callResult|.\[[Value]] to an IDL value of the same type as the operation’s
+        return type.
+    1.  <i id="construct-return">Return:</i> at this
+        point |completion| will be set to an ECMAScript completion value.
+        1.  [=Clean up after running a callback=] with |stored settings|.
+        1.  [=Clean up after running script=] with |relevant settings|.
+        1.  Return |completion|.

Yeah, we seem to mix that up all over the place. I guess we should define it more clearly. Maybe that's yet another thing for the infra standard?

-- 
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/328#pullrequestreview-28845645