- From: Anne van Kesteren <notifications@github.com>
- Date: Mon, 02 Mar 2020 05:15:24 -0800
- To: whatwg/encoding <encoding@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <whatwg/encoding/pull/198/review/367182653@github.com>
annevk commented on this pull request. Thanks for taking this on @andreubotella. Some suggestions for if you want to take this even further. > @@ -879,7 +879,7 @@ fallback encoding <var>encoding</var>, run these steps: <ol> <li><p>Let <var>buffer</var> be an empty byte sequence. - <li><p>Let <var>BOM seen flag</var> be unset. + <li><p>Let <var>BOM seen</var> be false. Can we make this `BOMSeen` while changing this? These days the convention seems to be to not use spaces for local variables. > @@ -1139,8 +1139,8 @@ attribute's getter, when invoked, must return true if this object's <p>The <dfn attribute id=dom-textdecoder-ignorebom for=TextDecoderCommon><code>ignoreBOM</code></dfn> -attribute's getter, when invoked, must return true if this object's -<a for=TextDecoderCommon>ignore BOM flag</a> is set, and false otherwise. +attribute's getter, when invoked, must return the value of this object's We can also use "`<a>this</a>'s`" rather than "this object's" now. > @@ -1234,14 +1233,13 @@ constructor, when invoked, must run these steps: method, when invoked, must run these steps: <ol> - <li><p>If the <a for=TextDecoder>do not flush flag</a> is unset, set <a for=TextDecoder>decoder</a> - to a new <a for=TextDecoderCommon>encoding</a>'s <a for=/>decoder</a>, set - <a for=TextDecoder>stream</a> to a new <a for=/>stream</a>, and unset the - <a for=TextDecoderCommon>BOM seen flag</a>. + <li><p>If <a for=TextDecoder>do not flush</a> is false, set <a for=TextDecoder>decoder</a> Another problem with this text is that it doesn't say where to grab "do not flush" from. See #175 for an idea of how to address that. Although I guess I'd accept it if you don't want to take that on as well. > <a lt="serialize stream" for=TextDecoderCommon>serialized</a>. - <p class=note>The way streaming works is to not handle <a>end-of-stream</a> here when the - <a for=TextDecoder>do not flush flag</a> is set and to not unset that flag. That way in a + <p class=note>The way streaming works is to not handle <a>end-of-stream</a> here when + <a for=TextDecoder>do not flush</a> is true and to not set that slot to false. That way in a s/that slot/it/ would work? Or duplicate the term. -- 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/encoding/pull/198#pullrequestreview-367182653
Received on Monday, 2 March 2020 13:15:36 UTC