Skip to content
This repository has been archived by the owner on Feb 18, 2025. It is now read-only.

Spec Review #60

Closed
9 of 10 tasks
jridgewell opened this issue Sep 27, 2023 · 5 comments
Closed
9 of 10 tasks

Spec Review #60

jridgewell opened this issue Sep 27, 2023 · 5 comments

Comments

@jridgewell
Copy link
Member

jridgewell commented Sep 27, 2023

  • Why do we need a "ASCII punctuators that need escaping" phrase?
  • Nit: rename cuList to something else. Maybe escapedList?
  • > If c is the first code point in cpList and c is a DecimalDigit, then
    • I don't see the phrase "is the first … in" anywhere else. I think we can update that to state "if cuList is empty" and use a defined "is empty" phrase and keep the same behavior.
  • There are several references to cuList that need to become _cuList_
  • > Append the elements of the UTF16Encoding (10.1.1) of c to cuList
    • What is UTF16Encoding? That 10.1.1 is 10.1.1 [[GetPrototypeOf]] ( )
  • There are several 0x00XY references "unicode" references, shouldn't they all be U+00XY?
  • DecimalDigit and WhiteSpace should be |DecimalDigit| and |WhiteSpace|
  • > and c is a DecimalDigit
    • c is a code point, can you compare it to a DecimalDigit production?
  • u> and c is a WhiteSpace
    • c is a code point, can you compare it to a WhiteSpace production?
  • > c is a CharSetElement of toEscape
    • c is a code point needs to be cast to a CharSetElement before we can check if it's an element of the set.
@jridgewell jridgewell mentioned this issue Sep 27, 2023
33 tasks
ljharb added a commit that referenced this issue Sep 28, 2023
@ljharb
Copy link
Member

ljharb commented Sep 28, 2023

Why do we need a "ASCII punctuators that need escaping" phrase?

i suppose i could do it inline, but i'd still want a note to describe the list a bit, and the dfn seemed cleaner to me.

ljharb added a commit that referenced this issue Oct 12, 2023
ljharb added a commit that referenced this issue Oct 12, 2023
@ljharb
Copy link
Member

ljharb commented Oct 12, 2023

@jridgewell please rereview; i've addressed all your comments.

@jridgewell
Copy link
Member Author

Everything in my original list looks good. But a new thing I've noticed, we're appending both Code Units and Strings to escapedList:

- Append code unit U+005C (REVERSE SOLIDUS) to escapedList.
- …
- Append the elements of [UTF16EncodeCodePoint](https://tc39.es/ecma262/#sec-utf16encodecodepoint)(c) to escapedList.

@ljharb
Copy link
Member

ljharb commented Oct 19, 2023

@jridgewell thanks, fixed! lmk if there's more items, or if i can check you off :-)

@jridgewell
Copy link
Member Author

LGTM

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants