Skip to content

Bug fix #6666#6726

Merged
existentialism merged 1 commit intobabel:masterfrom
rajasekarm:bugfix-6666
Nov 2, 2017
Merged

Bug fix #6666#6726
existentialism merged 1 commit intobabel:masterfrom
rajasekarm:bugfix-6666

Conversation

@rajasekarm
Copy link
Copy Markdown
Member

@rajasekarm rajasekarm commented Nov 2, 2017

Q                       A
Fixed Issues? #6666
Patch: Bug Fix?
Major: Breaking Change? No
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR
Any Dependency Changes?
License MIT

Edit by @nicolo-ribaudo:
Old PR: babel/babylon#778

@rajasekarm rajasekarm changed the title bug fix 6666 with test case Bug fix #6666 Nov 2, 2017
@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented Nov 2, 2017

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/5611/

@existentialism existentialism added pkg: parser PR: Bug Fix 🐛 A type of pull request used for our changelog categories area: flow labels Nov 2, 2017
}

if (next === 61) {
if (next === 61 && !exprAllowed) {
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.

(This has nothing to do with your PR)

The char codes are very difficult to read to me since I need to lookup in the table. It's sometimes inconsistent between int or hex representation.

I would like to PR to add a constant file where the char codes are stored and replace every occurrence in Babylon. What do you think?

Copy link
Copy Markdown
Member Author

@rajasekarm rajasekarm Nov 2, 2017

Choose a reason for hiding this comment

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

This seems to be a good idea which will make the code readable. I'll make it is as a separate PR.

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.

I would wait for more thoughts about since, especially @danez which is the main contrib of the parse.

Actually I already started locally. I can push a PR today, we can also both work on it.

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.

good idea

@existentialism existentialism merged commit 2d0211a into babel:master Nov 2, 2017
@existentialism
Copy link
Copy Markdown
Member

@rajzshkr thanks!

@rajasekarm rajasekarm deleted the bugfix-6666 branch January 11, 2018 09:21
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: flow outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Bug Fix 🐛 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants