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

Fix formatting of String Array literals containing right parens #4297

Conversation

Fryguy
Copy link
Contributor

@Fryguy Fryguy commented Apr 14, 2017

This fixes a bug where the formatter will automatically replace the
String Array literal tokens with parens regardless of whether the content
already contains a ). In the case where a ) already exists, the
resultant code is not compilable. This patch keeps the author's original
choice of token if a ) exists in the content, as they made a conscious
decision to choose that alternate token so that the code can compile.
Otherwise, the formatter will choose parens for code consistency as was
done previously.

For example,

%w{
  one)
  two)
}

would have previously had the curly braces replaced with parens, which
would no longer compile. Now, it will stay with curly braces, and be
properly formatted.

This fixes a bug where the formatter will automatically replace the
String Array literal tokens with parens regardless of whether the content
already contains a `)`.  In the case where a `)` already exists, the
resultant code is not compilable.  This patch keeps the author's original
choice of token if a `)` exists in the content, as they made a conscious
decision to choose that alternate token so that the code can compile.
Otherwise, the formatter will choose parens for code consistency as was
done previously.

For example,

```
%w{
  one)
  two)
}
```

would have previously had the curly braces replaced with parens, which
would no longer compile.  Now, it will stay with curly braces, and be
properly formatted.
@asterite asterite closed this in a8fbfb0 Apr 16, 2017
@asterite
Copy link
Member

@Fryguy Thanks!

The formatter always used %w(...), but not because of consistency: it was a bug. I fixed it to use the delimiter the user chose.

@Fryguy
Copy link
Contributor Author

Fryguy commented Apr 17, 2017

@asterite Thanks! Very interesting. I had assumed the Crystal core team chose parens as the default style since the formatter is opinionated (I actually prefer if the formatter is more opinionated...one less thing to bikeshed about)

@asterite asterite added this to the Next milestone Apr 18, 2017
@asterite
Copy link
Member

@Fryguy The formatter is opinionated regarding whitespace, but it never changes visible user input. For example do ... end is always kept like that, and so on.

@Fryguy Fryguy deleted the fix_format_string_array_literal_with_paren branch April 19, 2017 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants