The Wayback Machine - https://web.archive.org/web/20200917110622/https://github.com/atom/symbols-view/pull/157
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

Add es7 async functions to ctags #157

Merged
merged 3 commits into from Mar 8, 2016
Merged

Add es7 async functions to ctags #157

merged 3 commits into from Mar 8, 2016

Conversation

@thedaniel
Copy link
Contributor

thedaniel commented Feb 26, 2016

@atom/feedback ...any reason not to add these to the symbols-view? I've been writing a lot of babelized classes with async/await lately and this definitely makes my life better.

@thedaniel
Copy link
Contributor Author

thedaniel commented Feb 26, 2016

Actually, this regex might not be quite right since it needs to support default parameters as well. will update...

@joshaber
Copy link
Contributor

joshaber commented Feb 26, 2016

❤️

@lee-dohm
Copy link
Member

lee-dohm commented Feb 26, 2016

Sounds like a good idea to me 👍

@maxbrunsfeld
Copy link
Contributor

maxbrunsfeld commented Feb 26, 2016

👍

@thedaniel
Copy link
Contributor Author

thedaniel commented Mar 5, 2016

I changed the second character class in the function parameter capture group to .+ because writing a giant highly specific character class seems unnecessary given how specific the rest of the regex is. It now matches all of the following strings:

async connectedToDB () {

async request (requestPath, _fetchOptions = {}, _options = {}) {

async write (key, record, timestamp) {

I'd love to hear why I shouldn't be using the bludgeon that is .+ because while my regex is strong it is not kung fu master level, but so far this ctags-config is working for me on a babelized js project.

@lee-dohm
Copy link
Member

lee-dohm commented Mar 7, 2016

I'd love to hear why I shouldn't be using the bludgeon that is .+

I don't think it would be a big problem. Is there a reason you chose something other than the \([^)]*\) pattern for the parameter list like in the function version a couple lines previous?

@mnquintana
Copy link
Member

mnquintana commented Mar 7, 2016

Side note – it's a bit of a bummer that we can't reuse tokenization from the language- packages for generating symbols. 😞

@thedaniel
Copy link
Contributor Author

thedaniel commented Mar 8, 2016

@lee-dohm we shouldn't omit ) because we can supply a function call as a default param (i doubt this is common, but it does transpile successfully in babel so...)

cf:

class Foo {
   async functionWithFunctionCallAsDefaultParam (foo, bar = baz()) {
  }
}
@mnquintana
Copy link
Member

mnquintana commented Mar 8, 2016

@thedaniel Whoa, can you stick any expression in a default param?

@lee-dohm
Copy link
Member

lee-dohm commented Mar 8, 2016

Ah interesting. Thanks for clarifying @thedaniel 😀

@thedaniel
Copy link
Contributor Author

thedaniel commented Mar 8, 2016

merging, i really think .+ is fine but we can write out a large specific character class if we hit any bugs.

thedaniel added a commit that referenced this pull request Mar 8, 2016
Add es7 async functions to ctags
@thedaniel thedaniel merged commit d690405 into master Mar 8, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@thedaniel thedaniel deleted the dh-add-async-to-js branch Mar 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.