Fix formatting of lib's fun starting with uppercase letter #5434
Conversation
bew
force-pushed
the
fix-uppercase-fun-formatting
branch
from
f5eb49d
to
64a7a3b
Compare
| @@ -581,6 +581,8 @@ describe Crystal::Formatter do | |||
| assert_format "lib Foo\nend" | |||
| assert_format "lib Foo\ntype Foo = Bar\nend", "lib Foo\n type Foo = Bar\nend" | |||
| assert_format "lib Foo\nfun foo\nend", "lib Foo\n fun foo\nend" | |||
| assert_format "lib Foo\nfun Bar\nend", "lib Foo\n fun Bar\nend" | |||
| assert_format "lib Foo\nfun bar = Bar\nend", "lib Foo\n fun bar = Bar\nend" | |||
There was a problem hiding this comment.
What about lib Foo\nfun Foo = Bar\nend?
We should probably spec calling functions with CONST names too to make sure that doesn't get broken.
There was a problem hiding this comment.
@RX14 about calling, do you mean add a formatter spec for the lib-uppercase-fun calling?
bew
force-pushed
the
fix-uppercase-fun-formatting
branch
from
64a7a3b
to
2e395ea
Compare
| write node.name | ||
| next_token_skip_space | ||
| case @token.type | ||
| when :IDENT, :CONST |
There was a problem hiding this comment.
Maybe it would be better to let check receive multiple arguments and check for any of them?
There was a problem hiding this comment.
was thinking about it too, will probably do that
There was a problem hiding this comment.
Is this needed anywhere inside formatter.cr? If not, put it in this PR, if it is used in other places, do what those places do and refactor in a new PR.
There was a problem hiding this comment.
Good idea, it looks like it was the only place where a case @token.type was used this way. So I'll leave this change in this PR.
| def check(token_type) | ||
| raise "expecting #{token_type}, not `#{@token.type}, #{@token.value}`, at #{@token.location}" unless @token.type == token_type | ||
| def check(*token_types) | ||
| raise "expecting #{token_types.join " or "}, not `#{@token.type}, #{@token.value}`, at #{@token.location}" unless token_types.includes? @token.type |
There was a problem hiding this comment.
Splitting this from one line is overdue now 馃槃
There was a problem hiding this comment.
Just change the postfix unless to normal unless and it's readable enough.
bew
force-pushed
the
fix-uppercase-fun-formatting
branch
from
df008b3
to
ba7daa9
Compare
bew
force-pushed
the
fix-uppercase-fun-formatting
branch
from
befd98b
to
ca312d1
Compare
|
I squashed the intermediate commits so it should be merge-able as is |
|
Great first formatter contribution! =) |
|
@straight-shoota thanks! I'm glad to have indirectly helped the windows stuff going on these days, by unblocking your pr 馃槃 |
Fixes #5432
This is my first PR on the formatter 馃帀 馃槂