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

allow single line function definitions #1

Closed
wants to merge 3 commits into from
Closed

Conversation

@nlf
Copy link
Member

nlf commented May 12, 2015

this is primarily to cut down on noise in the hapi tests where many handler functions are declared on a single line

nlf added 2 commits May 12, 2015
this is primarily to cut down on noise in the hapi tests where many handler functions are declared on a single line
@nlf nlf mentioned this pull request May 12, 2015
@Marsup

This comment has been minimized.

Copy link
Member

Marsup commented May 12, 2015

For reference, we discussed this in hapijs/eslint-plugin-hapi#2. (I know wrong repo, sorry :))

@@ -13,7 +13,9 @@ module.exports = function(context) {
var bodyStartLine = body[0].loc.start.line;
var openBraceLine = context.getTokenBefore(body[0]).loc.start.line;

if (bodyStartLine - openBraceLine < 2) {
if (bodyStartLine !== openBraceLine &&

This comment has been minimized.

Copy link
@cjihrig

cjihrig May 13, 2015

Collaborator

I think you have to check the function's closing brace to verify that it's a one liner. For example, this should be a violation:

function foo() { var bar = 0;
  return bar;
}

You might also want to validate that the body's length is 0 or 1.

if (bodyStartLine - openBraceLine < 2) {
if (bodyStartLine !== openBraceLine &&
bodyStartLine - openBraceLine < 2) {

This comment has been minimized.

Copy link
@cjihrig

cjihrig May 13, 2015

Collaborator

Please remove this blank line.

@cjihrig

This comment has been minimized.

Copy link
Collaborator

cjihrig commented May 13, 2015

In hapijs/eslint-plugin-hapi#2, it was decided that one liners wouldn't be supported. However, I'd like to make it a configurable option of this rule. @nlf you're welcome to take a shot at making it configurable. Otherwise, I'm going to do it. You still have to talk to @geek about it being enabled in lab though.

@nlf

This comment has been minimized.

Copy link
Member Author

nlf commented May 13, 2015

i cleaned up the checks to verify a body.length of 1 and to compare the location of the closing brace, i can look at making it optional too but it'll be later today before i have a chance.

@cjihrig

This comment has been minimized.

Copy link
Collaborator

cjihrig commented May 19, 2015

Landed in 3fed70a. I'm going to work on making the option configurable (defaulting to not allowed) before I make a new release.

@cjihrig cjihrig closed this May 19, 2015
@cjihrig cjihrig added the feature label May 19, 2015
@cjihrig cjihrig added this to the v1.1.0 milestone May 19, 2015
@cjihrig cjihrig self-assigned this May 19, 2015
@cjihrig

This comment has been minimized.

Copy link
Collaborator

cjihrig commented May 19, 2015

Version 1.1.0 has been released. You can now opt in to one liners, and configure the number of statements allowed in one liners. The default values are what is currently in use in lab.

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