Skip to content

fixes #606: allow unquoted reserved words in more positions - #699

Merged
paf31 merged 2 commits into
purescript:masterfrom
michaelficarra:GH-606
Nov 11, 2014
Merged

fixes #606: allow unquoted reserved words in more positions#699
paf31 merged 2 commits into
purescript:masterfrom
michaelficarra:GH-606

Conversation

@michaelficarra

Copy link
Copy Markdown
Contributor

My first non-trivial pull request 😄. Fixes #606. Allows reserved words to be used in record literal keys, property updates, property accessors, destructurings, and record type signatures.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.01%) when pulling 420d012 on michaelficarra:GH-606 into 85f0cd1 on purescript:master.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could do this as lexeme $ (:) <$> identStart <*> many identLetter I think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah that should work. Would that be clearer?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure it makes much of a difference either way, but I think we use applicative style in the parser most of the time at the moment whenever it's not ridiculous. I could be wrong though, just going by memory!

@garyb

garyb commented Nov 10, 2014

Copy link
Copy Markdown
Member

Looks good to me! 👍

@michaelficarra

Copy link
Copy Markdown
Contributor Author

How about this? I borrowed the name ident from the internal parser in Parsec that has the same purpose.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.0%) when pulling 5811110 on michaelficarra:GH-606 into 85f0cd1 on purescript:master.

@garyb

garyb commented Nov 10, 2014

Copy link
Copy Markdown
Member

Sorry to be picky, but maybe we should call it propIdent or something instead? Might help avoid future mistakes using ident where it should be the current identifier.

@paf31

paf31 commented Nov 10, 2014

Copy link
Copy Markdown
Contributor

Yeah, I think I agree about ident. Otherwise, this looks great.

@michaelficarra

Copy link
Copy Markdown
Contributor Author

Sure, I will change it back to identifierName after the ECMA-262 grammar production with the same purpose.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.0%) when pulling 116668c on michaelficarra:GH-606 into 85f0cd1 on purescript:master.

@paf31

paf31 commented Nov 11, 2014

Copy link
Copy Markdown
Contributor

This is good to go, right?

@michaelficarra

Copy link
Copy Markdown
Contributor Author

Yep.

@paf31

paf31 commented Nov 11, 2014

Copy link
Copy Markdown
Contributor

How many libraries actually use the existing string literals? Can't be many.

@michaelficarra

Copy link
Copy Markdown
Contributor Author

One of mine does. But the string literals are still allowed. This just makes it more permissive for un-quoted names.

paf31 added a commit that referenced this pull request Nov 11, 2014
fixes #606: allow unquoted reserved words in more positions
@paf31
paf31 merged commit 12e296e into purescript:master Nov 11, 2014
@paf31

paf31 commented Nov 11, 2014

Copy link
Copy Markdown
Contributor

Gotcha, thanks.

@garyb

garyb commented Nov 11, 2014

Copy link
Copy Markdown
Member

Congrats @michaelficarra 😃

@michaelficarra
michaelficarra deleted the GH-606 branch August 17, 2015 17:23
hdgarrood added a commit that referenced this pull request Dec 17, 2018
Fixes #3482. We have allowed reserved names (like `data` or `type`) to
be used as record labels without being quoted since #699 was resolved.
This commit makes the `isUnquotedKey` function aware of this, having the
effect that these labels are no longer unnecessarily quoted in error
messages. For example, if we enter

    > {} :: { data :: Int }

in the repl, we now receive:

    Error found:

      Type of expression lacks required label data.

    while checking that expression {}
      has type { data :: Int
               }

whereas previously, `data` would have been quoted.
natefaubion pushed a commit that referenced this pull request Dec 21, 2018
Fixes #3482. We have allowed reserved names (like `data` or `type`) to
be used as record labels without being quoted since #699 was resolved.
This commit makes the `isUnquotedKey` function aware of this, having the
effect that these labels are no longer unnecessarily quoted in error
messages. For example, if we enter

    > {} :: { data :: Int }

in the repl, we now receive:

    Error found:

      Type of expression lacks required label data.

    while checking that expression {}
      has type { data :: Int
               }

whereas previously, `data` would have been quoted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

allow unquoted keywords as key names in record literals

4 participants