Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collapse grammar nonterminals that are only distinct for the purposes of disambiguation #2006

Open
ptomato opened this issue Jan 18, 2022 · 3 comments · May be fixed by #2767
Open

Collapse grammar nonterminals that are only distinct for the purposes of disambiguation #2006

ptomato opened this issue Jan 18, 2022 · 3 comments · May be fixed by #2767
Assignees
Labels
editorial has-consensus iso8601-grammar ISO 8061 grammar spec-text Specification text involved
Milestone

Comments

@ptomato
Copy link
Collaborator

ptomato commented Jan 18, 2022

@gibson042 pointed out that we don't need to have separate nonterminals such as TimeZoneNumericUTCOffset and TimeZoneUTCOffsetName in order to disambiguate in cases such as #1796. We can refer to things like "the first |FractionalPart|, if present", cf. https://tc39.es/ecma262/multipage/ecmascript-language-statements-and-declarations.html#sec-runtime-semantics-forloopevaluation

@gibson042
Copy link
Collaborator

gibson042 commented May 10, 2022

Related improvements are also possible with nonterminal parameterization, e.g.

       TimeZoneNumericUTCOffset :
-          TimeZoneUTCOffsetSign TimeZoneUTCOffsetHour
-          TimeZoneUTCOffsetSign TimeZoneUTCOffsetHour `:` TimeZoneUTCOffsetMinute
-          TimeZoneUTCOffsetSign TimeZoneUTCOffsetHour TimeZoneUTCOffsetMinute
-          TimeZoneUTCOffsetSign TimeZoneUTCOffsetHour `:` TimeZoneUTCOffsetMinute `:` TimeZoneUTCOffsetSecond TimeZoneUTCOffsetFraction? 
-          TimeZoneUTCOffsetSign TimeZoneUTCOffsetHour TimeZoneUTCOffsetMinute TimeZoneUTCOffsetSecond TimeZoneUTCOffsetFraction?
+          Sign HourComponents[+Extended]?
+          Sign HourComponents[~Extended]?
+
+      HourComponents[Extended] :
+          Hour
+          Hour TimeSeparator[?Extended] Minute
+          Hour TimeSeparator[?Extended] Minute TimeSeparator[?Extended] Second Fraction?
+
+      TimeSeparator[Extended] :
+          [+Extended] `:`
+          [~Extended] [empty]

@ptomato ptomato added the iso8601-grammar ISO 8061 grammar label Aug 11, 2022
@ptomato ptomato added this to the Stage "3.5" milestone Dec 8, 2022
@justingrant
Copy link
Collaborator

Offset grammar will be different between ISO string offsets and offset time zone identifiers after the minutes PR is merged. Does that change any of the simplifications planned here?

@gibson042
Copy link
Collaborator

Not really; UTCOffset nonterminals are now distinct for a legitimate reason and already incorporate parameterization as appropriate. But I still think some improvements are possible with respect to e.g. |TimeHour|/|TimeMinute|/|TimeFraction|/|TimeSpec| and the Duration productions.

@ptomato ptomato self-assigned this Feb 1, 2024
ptomato added a commit that referenced this issue Feb 2, 2024
Prefer language like "the first |TemporalDecimalFraction|, if present"
rather than defining several nonterminals such as |TimeFraction|,
|DurationHoursFraction|, etc.

Closes: #2006
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial has-consensus iso8601-grammar ISO 8061 grammar spec-text Specification text involved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants