Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Conversation

@mislav
Copy link
Contributor

@mislav mislav commented Sep 21, 2015

Supports proper tokenizing of these keywords when they appear as subexpression b within ternary expression a ? b : c:

yield, true, false, null, [CONSTANT], super, this, debugger,
module, exports, __filename, __dirname, global, process

They are not tokenized because there was a rule that these keywords should not be followed by :. This was to compensate for JS key/value syntax { foo: bar }.

The fix adds an alternate pattern for each of these keywords that explicitly allows these keywords to be followed by : if they were preceded by ?.

Fixes #93

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this should belong here? :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thanks

@winstliu
Copy link
Contributor

I dislike how we have to add duplicate values for all these rules - I'm guessing you already tried to combine just the non-capturing rules and failed?

Also, it feels like we can add two more describes specifically for object keys and ternaries, again to avoid duplication.

@mislav
Copy link
Contributor Author

mislav commented Sep 24, 2015

I dislike how we have to add duplicate values for all these rules - I'm guessing you already tried to combine just the non-capturing rules and failed?

Yes, I didn't have another idea. Basically we want to disallow these keywords being tokenized as such when they're followed by :, but only if they're not preceded with ?. I'm not sure if this can be described using a simpler regular expression.

Supports proper tokenizing of these keywords when they appear as
subexpression `b` within ternary expression `a ? b : c`:

  yield, true, false, null, [CONSTANT], super, this, debugger,
  module, exports, __filename, __dirname, global, process
@mislav mislav force-pushed the keywords-in-ternary branch from 6098da4 to 246bfea Compare September 24, 2015 17:20
@mislav
Copy link
Contributor Author

mislav commented Sep 24, 2015

I've performed the desired changes.

winstliu pushed a commit that referenced this pull request Sep 25, 2015
Fix tokenizing of various keywords within ternary expression
@winstliu winstliu merged commit d7b444a into atom:master Sep 25, 2015
@winstliu
Copy link
Contributor

Woo! 🍧

@BrandonZacharie
Copy link

👍

@Alhadis
Copy link
Contributor

Alhadis commented Sep 25, 2015

Mislav, you're awesome. If I were there I'd give ya the heartiest pat-on-the-back that I could give ya. :D 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

true and false don't get picked up inside ternary operator

4 participants