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

Indentation of callbacks #480

Closed
baygeldin opened this issue Mar 31, 2016 · 3 comments

Comments

@baygeldin
Copy link

commented Mar 31, 2016

Hi. This piece of code throws me an error of wrong indentation:

someFunction('very', 'very', 'long', 'parameters',
  'list', 'and', 'the', 'callback', function () {
  console.log('yep') // Expected indentation of 4 space characters but found 2.
}) // Expected indentation of 2 space characters but found 0.

While this is OK:

someFunction('very', 'very', 'long', 'parameters',
'list', 'and', 'the', 'callback', function () {
  console.log('yep')
})

However it's not very readable, especially when there are some real code with really long parameters written in multiple lines. I ran into this when I've been writing some middleware for koa.

I thought that maybe #208 is related to this, but it seems like it's something different.

@feross

This comment has been minimized.

Copy link
Member

commented Apr 1, 2016

This is probably a bug in eslint. If you're interested in getting this fixed, I recommend opening an issue on their bug tracker: https://github.com/eslint/eslint

The rule we use is: "indent": [2, 2, { "SwitchCase": 1 }]

Honestly, though, if I were you, I'd name the function in an argument list that long, so you can just refer to it in the list. Cleaner than inlining it.

@feross feross closed this Apr 1, 2016

@feross feross added the bug label Apr 1, 2016

@LinusU

This comment has been minimized.

Copy link
Member

commented Apr 1, 2016

Is it a bug though? I think that the first example looks wrong since the function body is indented the same as the parameter list. I think that it should be indented one more step...

someFunction('very', 'very', 'long', 'parameters',
  'list', 'and', 'the', 'callback', function () {
    console.log('yep')
  })
@baygeldin

This comment has been minimized.

Copy link
Author

commented Apr 1, 2016

Well, maybe it's a little far-fetched example. So here is the actual code:

fst.transition(INIT, [isCmd('get'), doHavePermissions(), 
isActive()], STARTED, function * (next) {
  // ...
})

It's a finite state transducer rule which says "If the state is INIT and the message complies to the list of filters, go to the state STARTED and execute following middleware". The array of filter can be long sometimes, but inlining it makes the rule easy to reason about. So, what I'm trying to say is that this scenario is still common.

Btw, I actually like the @LinusU suggestion. This is a way to go. However, allowing this as well as the second example from the post is not very intuitive.

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
3 participants
You can’t perform that action at this time.