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

Add Cop to Enforce Alignment of Tokens on Consecutive Lines #2478

Closed
jfelchner opened this issue Dec 7, 2015 · 8 comments
Closed

Add Cop to Enforce Alignment of Tokens on Consecutive Lines #2478

jfelchner opened this issue Dec 7, 2015 · 8 comments

Comments

@jfelchner
Copy link
Contributor

Now that ExtraSpacing has been updated (#2035) to work so well (thanks @bbatsov!), the last thing I'd love for rubocop to be able to handle is checking and autofixing alignment of tokens across consecutive lines.

For example, if the cop is configured to align on =, then the following code would throw an error:

hello = "my friend"
is_it_me ||= "you're lookin' for?"

whoomp = "there it is"

And autocorrect would update this to:

hello      = "my friend"
is_it_me ||= "you're lookin' for?"

whoomp = "there it is"

Firstly, notice that the whoomp line doesn't get autocorrected because it's not on an adjacent line with the first two. Similar to ExtraSpacing, it might be nice to have an option to ignore blank lines when looking for adjacency.

Secondly, it aligns on the token itself and not on the operator. In other words, it's the above and not:

hello    = "my friend"
is_it_me ||= "you're lookin' for?"
@alexdowad
Copy link
Contributor

Do you want this specifically for aligning on =? Or on other things as well?

This is not really aligning on "tokens", as ||= is parsed as a single tOP_ASGN token.

@jfelchner
Copy link
Contributor Author

@alexdowad I'm fairly sure that I haven't thought through all the use cases yet, so let's do whatever's easiest to align = (or other text? configurable?) across adjacent lines and then we can go from there as people use it more.

As a bonus, if it could do like the ExtraSpacing cop does where it looks on the preceding line at the same indentation level to determine where the alignment should go, that'd be pretty cool. That way, for example, the assignment of a multi line hash can line up with an assignment below the hash.

This is super minor, but I thought I would bring it up. 😄

@alexdowad
Copy link
Contributor

I think for now, we should only look at aligning =, and only in cases where there is one assignment statement per line, on successive lines. Otherwise, this will get completely out of hand.

@jfelchner
Copy link
Contributor Author

@alexdowad for a first pass I agree, although I don't think the indentation thing will be too hard. I'm down to pair on this as well as the bracket alignment with you if you want! :)

@jfelchner
Copy link
Contributor Author

@bbatsov What's up? Why'd this get closed?

@alexdowad
Copy link
Contributor

@jfelchner Have a look at the above commit.

@jfelchner
Copy link
Contributor Author

@alexdowad I'm on ioctocat and the commits don't get shown inline. :-/

Thank you so much! 😄

@alexdowad
Copy link
Contributor

No problem -- please clone and try it out! (It is better if we can catch problems before the next gem release.)

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

2 participants