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

Resolving #810: ES6 Object Literal Shorthand Detection #812

Merged
merged 1 commit into from
Nov 19, 2015

Conversation

timothyeburke
Copy link
Contributor

I had to update some of the decode tests - they no longer made sense once wrapped in the second part of the test and were failing. Let me know if this works for you.

@bitwiseman
Copy link
Member

I'm not opposed to test changes in general, but I don't understand why the change to the unencode tests is needed. I don't think that code path is touched by your change.

Other than that, it looks reasonable to me. There's also the myProperty() { ... }, syntax to contend with, but if you want to open a separate issue and solve that later, that'd be fine.

@@ -1196,6 +1196,17 @@ exports.test_data = {
')'
]
},
{
comment: "Issue 810 - es6 object literal detection",
unchanged: [
Copy link
Member

Choose a reason for hiding this comment

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

You might as well put both inputs that you listed in the issue report in here. They were just a little more interesting than this - worth covering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do that.

@timothyeburke
Copy link
Contributor Author

As for the updated test - it's because that test is currently in the format:

"foo", 'bar', "baz"

and on https://github.com/beautify-web/js-beautify/blob/master/test/template/python-javascript.mustache#L1237 that input gets wrapped in a { } block, meaning it is now

{ "foo", 'bar', "baz" }

and then beautified and treated as an object literal. Replacing those commas with semicolons changes the test to treat them as single line expressions rather than part of an es6 object literal.

@bitwiseman
Copy link
Member

Ah, okay. The javascript version of the test passed?
If so, then change the content back for both test and in the python test mustache change bt(...) to test_fragment(...). That will avoid the { } wrapping.

Also, as a last point, an you take your changes and rebase them into one commit?

Great work on this, BTW. Thanks!

@timothyeburke
Copy link
Contributor Author

Done and done! My team currently uses this (as part of grunt-jsbeautifier) for our es5 files, but I want to get through the issues that are keeping us from using it for our es6. Glad I can help, and thanks for helping me through this one.

bitwiseman added a commit that referenced this pull request Nov 19, 2015
Resolving #810: ES6 Object Literal Shorthand Detection
@bitwiseman bitwiseman merged commit ee03546 into beautifier:master Nov 19, 2015
@bitwiseman
Copy link
Member

@timothyeburke - High quality and low friction. Glad to have you do anymore fixes you choose. 😄

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.

2 participants