-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Bugfix: collection literals indentation in formatter #3959
Bugfix: collection literals indentation in formatter #3959
Conversation
"hahah⊙ⓧ⊙" => "aGFoYWjiipnik6fiipk=\n"} | ||
"Now is the time for all good coders\nto learn Crystal" => "Tm93IGlzIHRoZSB0aW1lIGZvciBhbGwgZ29vZCBjb2RlcnMKdG8gbGVhcm4g\nQ3J5c3RhbA==\n", | ||
"This is line one\nThis is line two\nThis is line three\nAnd so on...\n" => "VGhpcyBpcyBsaW5lIG9uZQpUaGlzIGlzIGxpbmUgdHdvClRoaXMgaXMgbGlu\nZSB0aHJlZQpBbmQgc28gb24uLi4K\n", | ||
"hahah⊙ⓧ⊙" => "aGFoYWjiipnik6fiipk=\n"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like here =>
one space righter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this formatter modifications just care about the alignment of the beginning of the expressions. The arrows there are not perfectly aligned just because the code before running the formatter was not perfectly aligned :-)
If you were suggesting to change this particular line in order to align the arrows, I'll be glad to do it. On the other hand, I'm not sure the formatter should be in charge of aligning the arrows. That should probably be discussed in another ticket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK it's because of variable-width encoding of utf-8 standard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes in code this looks ok, this is just github formatting
This looks really good to me! So 👍 for merging it. |
@juanedi could you rebase on master so the ci can check that the latest changes are still formatted accordingly to this PR? |
When the collection literal doesn't begin with a newline, contents are now aligned at the same column as the starting token, so this: x = { foo: 1, bar: 2 } ... turns into this: x = {foo: 1, bar: 2} When a newline is present, keep indentation as before (ie. indent one level deeper). This means this: x = { foo: 1, bar: 2 } ... turns into this: x = { foo: 1, bar: 2, }
b38d686
to
bd1330d
Compare
Fixes #3915
Changed formatter indentation for "collection" literals (arrays, hashes, tuples, ...)
When the expression doesn't begin with a newline, contents are now aligned at the same column as the starting token, so this:
... turns into this:
When a newline is present, keep indentation as before (ie. indent one level deeper). This means this:
... turns into this:
The reason for this behaviour is not to impose very long indentation offsets in expressions such as this one:
The only odd case would be this one:
... but I guess one could argue that such writing is inconsistent and we should either choose between newline at both the beginning and end of the expression or no newline at at. Enforcing this would be a bigger change.
Here is a summary of the specs I needed to change:
Before:
After:
Before:
After:
Before:
After:
Before:
After:
Before:
After:
Before:
After:
Before:
After: