Formatter: fixed private constants alignment #4112
Conversation
|
The new issue is here: #4113 |
|
I agree with @bew 's #4113 (comment) Let's add a blank line between different visibility constants. I can't think of a reason they should be visually together, and separating them seems to solve the alignment dilema in a trivial way. @makenowjust if you are willing to implement it, should we hold this PR as in WIP until both #4111 and #4113 are fixed together? |
|
@bcardiff See #4113 (comment). If segments should be separated by visibility modifiers, I think this PR is independent from #4113 problem, so you can merge this PR without waiting. I'm working to implement @bew's idea now. |
Fixed crystal-lang#4111 But now this file private Foo 1 A = 1 is formatted as: private Foo = 1 A = 1 I think it is too heavy to fix this problem.
bcardiff
force-pushed
the
fix/crystal-format/private-constant
branch
from
28b5715 to
eb1a7cd
Compare
bcardiff
added
the
kind:bug
|
@makenowjust I rebase and apply the formatter to some additional files due to rebase. It should be ok to merge upon CI. |
|
Thanks @makenowjust! Although it does not apply to the cases handled in this PR, this PR make me think if it is ok to force the format with respect the formatter in master (current behavior) or with the released formatter. The reason is that editor's plugin will use the later and it could be uncomfortable to the user that the saved file will trigger some changes... |
…mat/private-constant Formatter: fixed private constants alignment
Fixed #4111
But I appeared another problem. This file:
is formatted as:
I think it is too heavy to fix this problem, and we should not define mixed visibility constants in same segment in general. I will open a new issue for this problem.