Skip to content

Normalize :: syntax #5048

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

Merged
merged 2 commits into from
May 13, 2018

Conversation

helixbass
Copy link
Collaborator

@GeoffreyBooth a couple Prettier-related changes to try and make generation of :: constructs safer:

  • Currently, ., ?. and :: are LINE_CONTINUERs but ?:: is not. So add it so that line-breaking before any kind of Accessor (other than a "computed" one, ie an Index eg ['a']) should be safe
  • Currently, there is "standalone" :: (ie just referencing the .prototype object) as well as :: followed by a Property. ?:: has no standalone version in the grammar. So add it, since currently in Prettier I'm auto-transforming any .prototype access into ::, so if the original source were eg a?.prototype it'd generate a?::, which currently won't compile. If it ends up making sense to only generate :: when explicit :: was in the original source, that should just be a question of eg adding a prototypeShorthand: yes flag to the Access in the grammar, but there's no harm in allowing standalone ?:: regardless

@GeoffreyBooth
Copy link
Collaborator

@zdenko or @vendethiel ?

Copy link
Collaborator

@connec connec left a comment

Choose a reason for hiding this comment

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

Seems like a well-scoped change. Only one comment:

  • Would it be worth adding a test explicitly for soaked :: (e.g. test the soaking behaviour, rather than the line continuation) in soaks.coffee?

@helixbass
Copy link
Collaborator Author

@connec looks like there were existing checks that ?:: behaves as expected in test/operators.coffee, so i expanded that test to test that ?:: behaves the same way when continuing a line

@GeoffreyBooth GeoffreyBooth merged commit 41185ca into jashkenas:master May 13, 2018
@GeoffreyBooth GeoffreyBooth mentioned this pull request May 20, 2018
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.

3 participants