-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
ForceEqualSignAlignment shouldn't use method definition in alignment #2722
Comments
@alexdowad I think that one of the keys here is that, for reference of proper alignment, the cop needs to go up line by line until it gets to either:
In that way, both of these (currently broken) cases will be caught: Previous Line of Lesser Indentation Contains Tokenclass Foo
def here_is_a_long_method_name(options = {})
self.bar = 'cool'
self.bazinga = 'sweet'
end
end Should be corrected to: class Foo
def here_is_a_long_method_name(options = {})
self.bar = 'cool'
self.bazinga = 'sweet'
end
end Instead of: class Foo
def here_is_a_long_method_name(options = {})
self.bar = 'cool'
self.bazinga = 'sweet'
end
end Previous Line of Greater Indentation Does Not Contain Tokenclass Foo
def bar(options = {})
self.bazinga = {
one: '1',
two: '2',
}
self.bar = 'cool'
end
end Should be corrected to: class Foo
def bar(options = {})
self.bazinga = {
one: '1',
two: '2',
}
self.bar = 'cool'
end
end Instead of: class Foo
def bar(options = {})
self.bazinga = {
one: '1',
two: '2',
}
self.bar = 'cool'
end
end |
@jfelchner Good points. But I wonder if the suggestion from @Dbz should also be heeded. def method1(arg1, arg2 = :default); end
def method2(arg3 = :default); end It would look pretty strange to align the equals signs in those arglists, don't you think? |
@alexdowad in the rare cases where I have multiple single line methods, I dunno, I might actually like them aligned 😀 but honestly I could go either way with whatever you decide. 👍 You've been killing it lately man. Thank you so much for your work. |
I agree with Alex that it might seem awkward to align the two methods- especially because you would be aligning the second parameter from I also think there might be a bug where calling I'd be happy to pair with you @alexdowad on working out a solution to fix these cops if you're up for it. It would give me better insight to the codebase. |
Your suggested fix using |
Oh, I can definitely do the rather simple |
@alexdowad would there be any way, while you're in that cop, to make it handle my second use case above? Where the cop looks at the token placement on the previous line of equal indentation? |
@Dbz I re-read this issue and saw your example for the alignment of those method definitions with fresh eyes. I didn't even notice that the first one had two arguments hahahaha Definitely shouldn't align the equal signs on those. 😉 |
Running
rubocop -a
turns the followingdfs
method:into this
dfs
method:Rubocop shouldn't take the method definition into consideration when forcing equal sign alignment. I can fix this by adding or modifying the
.select
statement within thedef investigate(processed_source)
method in extra_spacing.rb.The text was updated successfully, but these errors were encountered: