Skip to content
This repository was archived by the owner on Jun 15, 2023. It is now read-only.

Dropped annotations from punned record labels. #584

Merged
merged 12 commits into from
Jun 27, 2022

Conversation

cristianoc
Copy link
Contributor

@cristianoc cristianoc commented Jun 27, 2022

Add examples of expression and patterns showing where they are dropped.
Support all the cases discussed in #583

Add examples of expression and patterns showing where they are dropped.

See #583
This is now produced by the printer but is a parsing error:
```rescript
| {@optional name, x: 3} => 4242
```
@cristianoc cristianoc force-pushed the annotate-record-field branch from 5b60f01 to d76a32f Compare June 27, 2022 03:33
@@ -2674,6 +2676,7 @@ and parseJsxChildren p =
and parseBracedOrRecordExpr p =
let startPos = p.Parser.startPos in
Parser.expect Lbrace p;
let attrsForFields = parseAttributesForFields p in
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very close to surface syntax.

@cristianoc cristianoc force-pushed the annotate-record-field branch from e8aa6b0 to 949a7b9 Compare June 27, 2022 06:40
@cristianoc cristianoc requested a review from cknitt June 27, 2022 06:40
@@ -6250,9 +6263,29 @@ and parseAttribute p =
] )
| _ -> None

and parseAttributeForFields p =
let isAttributeForField p =
Parser.lookahead p (function state ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the grammar forces us into using a lookahead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's super overloaded over there. Apparently {JSX stufff} only works because the JSX attribute drives the parser into a specific path (which does not consider records). Which itself is arbitrary.

I'm thinking of considering proper surface syntax early on, so one does not need to distinguish based on the name of attributes, which is really not robust.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this PR, it already distinguished based on the presence of attributes, which is also unclear whether intended.

@cristianoc cristianoc merged commit 677f225 into master Jun 27, 2022
@cristianoc cristianoc mentioned this pull request Jun 27, 2022
@cristianoc cristianoc deleted the annotate-record-field branch July 23, 2022 04:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants