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

Check indentation of method arguments spanning multiple lines #1144

Closed
bquorning opened this issue Jun 10, 2014 · 9 comments
Closed

Check indentation of method arguments spanning multiple lines #1144

bquorning opened this issue Jun 10, 2014 · 9 comments
Assignees

Comments

@bquorning
Copy link
Contributor

Rubocop with default configuration detects no offenses on this code:

def run(first, second = {})
  puts "#{first} #{second}"
end

def defaults
  { a: 1 }
end

run(:foo, defaults.merge(
   bar: 3
  )
 )

run(
   :foo,
   bar: 3
 )

I do, however. All of the arguments to the #run calls are mis-indented.

@bbatsov
Copy link
Collaborator

bbatsov commented Jun 12, 2014

@jonas054 Would you look into this?

@jonas054
Copy link
Collaborator

Sure. @bbatsov @bquorning What do you think the correct indentation should be? Some example code please.

@bbatsov
Copy link
Collaborator

bbatsov commented Jun 12, 2014

Perhaps:

run(:foo, defaults.merge(
            bar: 3
          )
   )

run(
  :foo,
  bar: 3
)

@bbatsov
Copy link
Collaborator

bbatsov commented Jun 12, 2014

Not sure, about the first case, but I'm pretty sure about the second one.

@bquorning
Copy link
Contributor Author

There’s no doubt about the second example – I would indent it as @bbatsov does:

run(
  :foo,
  bar: 3
)

The first one is more tricky. I definitely don’t like the “hanging indentation” of the hash and closing parentheses. The problem is that two parentheses are opened on the same line, but closed on different lines.

They should either be closed on the same line

run(:foo, defaults.merge(
  bar: 3
))

Or opened on different lines:

run(:foo,
  defaults.merge(
    bar: 3
  )
)

Or one or both sets of parentheses just be on the same line:

run(:foo,
  defaults.merge(bar: 3)
)

run(:foo, defaults.merge(bar: 3))

Keeping both opening parentheses on one line and closing them on different lines, I cannot think of a style that looks pleasant:

run(:foo, defaults.merge(
    bar: 3
  )
) # Nope

run(:foo, defaults.merge(
            bar: 3
          )
) # Nope

run(:foo, defaults.merge(
            bar: 3
          )
   ) # Nope

@jfelchner
Copy link
Contributor

Where the arguments are placed should be the focus of one cop and how the closing parens are placed should probably be the focus of a separate cop no?

@jonas054
Copy link
Collaborator

@jfelchner Yes! Because they follow different rules.

@jonas054 jonas054 self-assigned this Jan 26, 2015
jonas054 added a commit to jonas054/rubocop that referenced this issue Feb 1, 2015
Add a cop that checks the indentation of the first parameter
in a method call. Part of solution for rubocop#1144.
@bquorning
Copy link
Contributor Author

Great work on the FirstParameterIndentation cop.

Would it be possible to check the ending parentheses, too? RuboCop still reports no offenses for this code:

def foo
  puts('x'
                            )
end

def bar
  puts(
    'y'
                            )
end

@bbatsov
Copy link
Collaborator

bbatsov commented Feb 23, 2015

It's possible, but it should be done by a dedicated cop IMO.

On 23 February 2015 at 14:57, Benjamin Quorning notifications@github.com
wrote:

Great work on the FirstParameterIndentation cop.

Would it be possible to check the ending parentheses, too? RuboCop still
reports no offenses for this code:

def foo
puts('x'
)end
def bar
puts(
'y'
)end


Reply to this email directly or view it on GitHub
#1144 (comment).

Best Regards,
Bozhidar Batsov

http://www.batsov.com

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

No branches or pull requests

4 participants