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

Split source elements relative to their language. #3069

Merged
merged 3 commits into from Oct 22, 2017

Conversation

CiGit
Copy link
Member

@CiGit CiGit commented Oct 19, 2017

Fix #3068 : Colliding node types which are not source elements in every language.
Example: ObjectExpression in JSON / JS

Source Elements are now based on the parser.

Colliding node types which are not source elements in every language.
Example ObjectExpression in JSON / JS
index.js Outdated
case "flow":
case "babylon":
case "typescript":
return jsSourceElements.includes(node.type);
Copy link
Member Author

@CiGit CiGit Oct 19, 2017

Choose a reason for hiding this comment

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

Let's end tests, but I'm not sure .includes is in node 4...

case "ScalarTypeDefinition": // GraphQL
return true;
// JS and JS like to avoid repetitions
const jsSourceElements = [
Copy link
Member Author

@CiGit CiGit Oct 19, 2017

Choose a reason for hiding this comment

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

What's the rule with global variables? those const ...SourceElements could be defined outside.

a: 1,
},<<<PRETTIER_RANGE_START>>>
{
a: 1,
Copy link
Member

Choose a reason for hiding this comment

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

Could you add an additional property to this object? Otherwise it could be treated as a block + label.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, it's certainly clearer than trying to find a semicolon {a: 1;}.
Done.

ObjectExpression vs BlockStatement & LabeledStatement
case "UnionTypeDefinition": // GraphQL
case "ScalarTypeDefinition": // GraphQL
return true;
// JS and JS like to avoid repetitions
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this supposed to be "JS and TS like to avoid repetitions"?

Copy link
Member

Choose a reason for hiding this comment

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

I interpreted as JS-like = flow, TypeScript.

@josephfrazier
Copy link
Collaborator

josephfrazier commented Oct 21, 2017 via email

@azz azz merged commit e589bab into prettier:master Oct 22, 2017
@CiGit CiGit deleted the source-elements branch October 22, 2017 09:05
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jan 19, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants