Re: [whatwg/fetch] Define the Content-Type header parser (#831)

MattMenke2 commented on this pull request.

This all looks pretty good to me.

> +steps:
+
+<ol>
+ <li><p>Let <var>initialValue</var> be the result of <a for="header list">getting</a>
+ <var>name</var> from <var>list</var>.
+
+ <li><p>If <var>initialValue</var> is null, then return null.
+
+ <li><p>Let <var>input</var> be the result of <a>isomorphic decoding</a> <var>initialValue</var>.
+
+ <li><p>Let <var>position</var> be a <a for=string>position variable</a> for <var>input</var>,
+ initially pointing at the start of <var>input</var>.
+
+ <li><p>Let <var>values</var> be a <a for=/>list</a> of <a for=/>strings</a>, initially empty.
+
+ <li><p>Let <var>value</var> be null.

Do we get anything by starting with value as null instead of the empty string?  Seems like if we had it as the empty string, it saves a line of "code".  We do end up with values being an empty string at the end of the code, but I don't think that hurts?  The code is correct, either way, just think that less complexity is better, though don't have strong feelings here.

> +  <tr>
+   <td>
+    <pre><code>A: text/html;", x/x</code></pre>
+   <td rowspan=2>« "<code>text/html;", x/x</code>" »
+  <tr>
+   <td>
+    <pre><code>
+A: text/html;"
+A: x/x
+</code></pre>
+  <tr>
+   <td>
+    <pre><code>
+A: x/x;test="hi",y/y
+</code></pre>
+   <td rowspan=2>« "<code>x/x;test="hi"</code>", "<code>y/y</code>" »

This is correct, but distinguishing the code and non-code quotes can be a bit confusing.  Maybe remove the extra delimiters and put each value on a separate line instead?  Or use something other than quotes?

<"x/x;test="hi">, <y/y> (With or without the surrounding «»)


> + <p>This is how <a>extract a MIME type</a> functions in practice:
+
+ <table>
+  <tr>
+   <th>Headers (as on the network)
+   <th>Output
+  <tr>
+   <td>
+    <pre><code>
+Content-Type: text/plain;charset=gbk, text/html
+</code></pre>
+   <td><code>text/html</code>
+  <tr>
+   <td>
+    <pre><code>
+Content-Type: text/html;charset=gbk, text/html;x=y

Maybe add an ";a=b" to the first set, which shouldn't carry over?

> + <table>
+  <tr>
+   <th>Headers (as on the network)
+   <th>Output
+  <tr>
+   <td>
+    <pre><code>
+Content-Type: text/plain;charset=gbk, text/html
+</code></pre>
+   <td><code>text/html</code>
+  <tr>
+   <td>
+    <pre><code>
+Content-Type: text/html;charset=gbk, text/html;x=y
+</code></pre>
+   <td><code>text/html;x=y;charset=gbk</code>

The output is an essense and a parameters array, not a string, right?  i.e., there's nothing to say that this is more like:

text/html;x=y;charset=gbk

instead of:

text/html;charset=gbk;x=y

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