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

Sync with language-javascript #103

Merged
merged 50 commits into from Sep 4, 2017
Merged

Sync with language-javascript #103

merged 50 commits into from Sep 4, 2017

Conversation

50Wliu
Copy link
Contributor

@50Wliu 50Wliu commented Oct 9, 2016

This is a pretty large overhaul of language-coffee-script that aims to bring it up-to-date with language-javascript. Things I've ported over include:

  • Functions
  • Function calls
  • Method calls
  • Objects
  • Properties
  • Numbers
  • Operators
  • Begin/end for braces
Before After
coffee-script-before coffee-script-after

Fixes #18 (they should both actually be variable assignments and not functions)
Fixes #25
Fixes #45
Fixes #63
Fixes #79
Fixes #88
Fixes #90
Fixes #97
Fixes #111
Closes #53

TODO:

  • Instance variables aren't being tokenized correctly in parameters (see the fixme in the code).
  • Argument matching terminates at the end of the line instead of attempting to continue in the case of inline functions - Unfixable using regex
  • Similar to above, argument matching does not continue for multiline arguments, even if they're surrounded by parentheses - Unfixable using regex
  • Quoted functions are not recognized ("test": ->)
  • Objects turn into properties if you add an existence check (a.b?.c)
  • Properties don't turn into method calls if you add a standalone @ (console.log @)

/cc @atom/feedback - if some of you could try using this branch for everyday coding, that would be great in order to spot any regressions.
/cc @esdoppio @MaximSokolov @pchaigno

@50Wliu
Copy link
Contributor Author

50Wliu commented Oct 9, 2016

Remaining problems:

  • Instance variables aren't being tokenized correctly in parameters (see the fixme in the code).
  • Argument matching terminates at the end of the line instead of attempting to continue in the case of inline functions
  • Similar to above, argument matching does not continue for multiline arguments, even if they're surrounded by parentheses
  • Quoted functions are not recognized ("test": ->)

(this post will be continually edited)
moved to the main pull request description

@50Wliu
Copy link
Contributor Author

50Wliu commented Oct 9, 2016

Other than the "messed up" line in #53, everything else is now tagged. After this is merged I'll open a new issue for that.
Fixed in the commits below!

@pchaigno
Copy link
Contributor

pchaigno commented Oct 10, 2016

It's pretty great overall!
I found a couple constructs that seem broken by this:

  1. Example before (master) in Lightshow
  2. Example with this pull request

Notice a few keywords are not highlighted near the end.

@50Wliu
Copy link
Contributor Author

50Wliu commented Oct 10, 2016

Thanks - fixed!
Lightshow

'captures':
'0':
'name': 'variable.parameter.function.coffee'
'1': # FIXME: Doesn't work yet: the instance_variable include overrides this

Choose a reason for hiding this comment

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

It probably should be

{
  'match': '([a-zA-Z_$][\\w$]*)(\\.\\.\\.)?'
  'captures':
    '1':
      'name': 'variable.parameter.function.coffee'
    '2':
      'name': 'keyword.operator.splat.coffee'
}
{
  'match': '(@(?:[a-zA-Z_$][\\w$]*)?)(\\.\\.\\.)?'
  'captures':
    '1':
      'name': 'variable.parameter.function.readwrite.instance.coffee'
    '2':
      'name': 'keyword.operator.splat.coffee'
}
{
  'include': '$self'
}

'2':
'name': 'variable.other.readwrite.instance.coffee'
'3':
'name': 'keyword.operator.assignment.coffee'

Choose a reason for hiding this comment

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

Note that =, : operators now scoped differently

test = () -> x * x # keyword.operator.assignment
a = 1 + 1 # just keyword.operator

image
Seti syntax

Copy link
Contributor Author

@50Wliu 50Wliu Oct 11, 2016

Choose a reason for hiding this comment

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

Bah. Looks like I'll have to update those as well.

Edit: Scratch that, just going to change these scopes back to keyword.operator.

@MaximSokolov
Copy link

It also closes #64

@50Wliu
Copy link
Contributor Author

50Wliu commented Oct 10, 2016

To be clear, this suffers from the same problems in #64 as language-javascript also can't tokenize those functions.

@50Wliu
Copy link
Contributor Author

50Wliu commented Oct 11, 2016

Thanks @MaximSokolov - your fix did work!

Until we can port over the relevant language-javascript changes to
keyword matching.
Operators is still WIP.  Need to extract it into its own section in the
repository.  Just wanted to convert it as easily as possible first.

Also, started work on adding specs to cover all these changes.
@50Wliu 50Wliu mentioned this pull request Feb 7, 2017
@lee-dohm
Copy link
Contributor

lee-dohm commented Feb 7, 2017

I've been running this for a couple days and things look good to me 👍

@50Wliu
Copy link
Contributor Author

50Wliu commented Mar 15, 2017

console.log not hasScope(beforePrefixScopesArray, "entity.name.tag.css") and prefix.trim() is ":"

^ reminder for me to fix that

Edit: Fixed! deff726

@50Wliu
Copy link
Contributor Author

50Wliu commented Apr 5, 2017

stack is incorrectly tokenized as a function:

for currentStack in stack by -1

Edit: fixed in 55dec99

No support for ..:

index = parseInt(match[1..])

Edit: I won't be adding support for splices to this PR.

And add specs for for loops
@50Wliu
Copy link
Contributor Author

50Wliu commented Jun 16, 2017

error = new Error("Deprecated function(s) #{deprecations.map(({originName}) -> originName).join ', '}) were called.")

string termination breaks somewhere in there.

Edit: Fixed in 43d6b9d

@50Wliu 50Wliu merged commit 073f6a1 into master Sep 4, 2017
@50Wliu 50Wliu deleted the wl-sync-with-javascript branch September 4, 2017 13:19
@50Wliu 50Wliu mentioned this pull request Sep 4, 2017
@Ben3eeE
Copy link

Ben3eeE commented Sep 4, 2017

🎉

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