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

js-mode function snippet does not generate canonical javascript function syntax for anonymous functions #101

Closed
cowboyd opened this issue Aug 24, 2015 · 5 comments · Fixed by #104

Comments

@cowboyd
Copy link

cowboyd commented Aug 24, 2015

The javascript function snippet introduced here 99e72dd

When the name snippet parameter is skipped, the function.el snippet will generate a function expression like so

function (one, two, three) {
//function body
}

but by far the most common style is to not have a space in between the function keyword and parameter list for anonymous functions:

function(one, two, three) {
}

Almost any example you'll find on the web uses this style found here at mozdev on the function statement documentation:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function

@AndreaCrotti
Copy link
Owner

@cowboyd try that fix and I'll merge it if it's better..
I had to make the snippet slightly more complicated but it supports both anonymous and named function now.

The alternative could be make two snippets choosing with the same key, so choosing early on if it should be a named or anonymous function, what do you think?

@cowboyd
Copy link
Author

cowboyd commented Sep 10, 2015

Just saw this. I'll give it a try.

@cowboyd
Copy link
Author

cowboyd commented Sep 14, 2015

So after trying this for awhile, I really like this snippet, but I find myself missing the old behavior of just getting the quick anonymous function... especially, since this is something like 90% of the usecases.

What would you think of f being bound to the old behavior, and fn being a named function?

@AndreaCrotti
Copy link
Owner

I'm not too sure @cowboyd, because

  1. to get the anonymous function it's just one extra Ctrl-d now
  2. it would be confusing to everyone that is now used to use f TAB for functions
  3. from my understanding of JS anonymous functions are still more or less discouraged, since they don't help debugging and similar things

So I'll leave it like that for now I think..

@cowboyd
Copy link
Author

cowboyd commented Sep 22, 2015

I'll just throw in my final $.02:

  1. It would be confusing to everyone that is now used to use f TAB for functions

The original behavior change threw me for a loop as I had been using f for anonymous function for a long time.

  1. from my understanding of JS anonymous functions are still more or less discouraged, since they don't help debugging and similar things

I don't think this is supported by the evidence. If you look at most large and popular codebases, when assigning functions to a prototype or as a property in an object literal, or as an iterator in an each or forEach, anonymous functions are used. For example, here is a mixin from the React codebase https://github.com/facebook/react/blob/master/src/shared/utils/Transaction.js#L77

You'll find a similar situation in the Angular and Ember codebases to name some more high profile examples.

Of course, this is just a few examples of the myriad JavaScript repositories out there, but I think they are very representative of the mainstream, and maintained by organizations that place a premium on best practices and coding standards.

Whatever you decide, let me say thank you for maintaining and improving these snippets. They are very helpful, and improve the productivity of both me and my team. Cheers.

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 a pull request may close this issue.

2 participants