Re: [whatwg/encoding] Editorial: separate `TextDecoderStream` constructor to two algos (PR #364)

@annevk commented on this pull request.

Looks good, but I have a bunch of nits.

> @@ -1851,23 +1851,31 @@ constructor steps are:
 
  <li><p>If <var>encoding</var> is failure or <a>replacement</a>, then <a>throw</a> a {{RangeError}}.
 
- <li><p>Set <a>this</a>'s <a for=TextDecoderCommon>encoding</a> to <var>encoding</var>.
+ <li><p>Let <var>errorMode</var> be <code>fatal</code> if <var>options</var>["{{TextDecoderOptions/fatal}}"] is true; otherwise <code>replacement</code>.

fatal and replacement here need quotes.

>  
- <li><p>If <var>options</var>["{{TextDecoderOptions/fatal}}"] is true, then set <a>this</a>'s
- <a for=TextDecoderCommon>error mode</a> to "<code>fatal</code>".
+ <li><p>Let <var>ignoreBOM</var> be true if <var>options</var>["{{TextDecoderOptions/ignoreBOM}}"] is true; otherwise false.

```suggestion
 <li><p>Let <var>ignoreBOM</var> be <var>options</var>["{{TextDecoderOptions/ignoreBOM}}"].
```
But given that we can write it like this, we might as well inline it completely and not have a separate variable.

>  
- <li><p>Set <a>this</a>'s <a for=TextDecoderCommon>decoder</a> to a new instance of <a>this</a>'s
- <a for=TextDecoderCommon>encoding</a>'s <a for=/>decoder</a>, and set <a>this</a>'s
- <a for=TextDecoderCommon>I/O queue</a> to a new <a for=/>I/O queue</a>.
+<div algorithm>
+<p>To <dfn export>set up a text decoder stream</dfn> <var>stream</var>, given an optional [=/encoding=] <var>encoding</var> (default <a>UTF-8</a>),
+an optional <a for=/>error mode</a> <var>errorMode</var> (default <code>replacement</code>), and an optional boolean <var>ignoreBOM</var> (default false), run these steps:

```suggestion
an optional <a for=/>error mode</a> <var>errorMode</var> (default "<code>replacement</code>"), and an optional boolean <var>ignoreBOM</var> (default false), run these steps:
```

>  
- <li><p>Set <a>this</a>'s <a for=TextDecoderCommon>decoder</a> to a new instance of <a>this</a>'s
- <a for=TextDecoderCommon>encoding</a>'s <a for=/>decoder</a>, and set <a>this</a>'s
- <a for=TextDecoderCommon>I/O queue</a> to a new <a for=/>I/O queue</a>.
+<div algorithm>
+<p>To <dfn export>set up a text decoder stream</dfn> <var>stream</var>, given an optional [=/encoding=] <var>encoding</var> (default <a>UTF-8</a>),

Let's also type _stream_ while we're here. Maybe rephrased as:

> To set up a text decoder stream, given a TYPE _stream_, optional encoding _encoding_, ...

>  
- <li><p>Set <a>this</a>'s <a for=TextDecoderCommon>decoder</a> to a new instance of <a>this</a>'s
- <a for=TextDecoderCommon>encoding</a>'s <a for=/>decoder</a>, and set <a>this</a>'s
- <a for=TextDecoderCommon>I/O queue</a> to a new <a for=/>I/O queue</a>.
+<div algorithm>
+<p>To <dfn export>set up a text decoder stream</dfn> <var>stream</var>, given an optional [=/encoding=] <var>encoding</var> (default <a>UTF-8</a>),
+an optional <a for=/>error mode</a> <var>errorMode</var> (default <code>replacement</code>), and an optional boolean <var>ignoreBOM</var> (default false), run these steps:
+
+<ol>

Let's assert that _encoding_ is not replacement.

> @@ -1877,7 +1885,11 @@ constructor steps are:
  <a for="TransformStream/set up"><var ignore>flushAlgorithm</var></a> set to
  <var>flushAlgorithm</var>.
 
- <li><p>Set <a>this</a>'s <a for=GenericTransformStream>transform</a> to <var>transformStream</var>.
+ <li><p>Set <var>stream</var>'s <a for=TextDecoderCommon>decoder</a> to a new instance of <var>encoding</var>'s <a for=/>decoder</a>.
+
+ <li><p>Set <var>stream</var>'s <a for=TextDecoderCommon>I/O queue</a> to a new <a for=/>I/O queue</a>.

Any reason to move these two steps further down? Making them separate steps makes sense, but it would be nicer for the diff if they are in the same place still.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/encoding/pull/364#pullrequestreview-4321560142
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/encoding/pull/364/review/4321560142@github.com>