Allow range formation operators without surrounding spaces to wrap safely#1170
Allow range formation operators without surrounding spaces to wrap safely#1170TTOzzi wants to merge 1 commit into
Conversation
|
In the test example, let x =
(aaa + bbb)..<(ccc
+ ddd)I think I would prefer the line break to occur before the Here's an interesting idea that we could try: what if we let the break happen before the range formation operator, but we change the rule to be that if that break occurs (don't respect discretionary breaks here), then we also insert a space after the operator? That would produce much better breaking than the current approach that forces the token to stay glued together. Then the spaces-around-range-formation-operators option would be more of a "best effort", which I think is fine; better formatting should trump rigidly following the option in this case. I'm not sure if the formatting token model actually supports that kind of conditional logic today, but I think it would be a cleaner way forward. |
c696f98 to
49b26cc
Compare
cb97715 to
5603729
Compare
5603729 to
177ce93
Compare
177ce93 to
d99be60
Compare
Thanks for the suggestion 🙇 I implemented this by extending the spacing model so a space token can carry conditional behavior. That allows range formation operators to remain unspaced when they stay on one line, while still inserting a space after the operator if we wrap before it. I also introduced I also added test cases covering the new wrapping behavior and its interaction with existing line break preservation. |
allevato
left a comment
There was a problem hiding this comment.
This is great! I like the way this turned out.
Can you take a look at the case that's failing in sourcekit-lsp? It looks like the continuation break is firing there when we should try to avoid it (because it's fine for the ..< to stick to the surrounding parentheses.
| // fires, we preserve the infix parse by inserting a space after the operator. | ||
| before( | ||
| binOp.firstToken(viewMode: .sourceAccurate), | ||
| tokens: .break(.continue, size: 0) |
There was a problem hiding this comment.
Can you make this break ignoresDiscretionary: true so that we don't allow breaking unless it's otherwise required? We don't want to encourage people to break around these operators in situations like this if the margin is much farther out:
for i in 0
..< 5
{Then add a test for a case like this to verify that we collapse the newline and remove the space on the other side.
Resolve #1141
Range operators in parenthesized infix expressions could wrap before the operator when formatted without surrounding spaces, changing parsing by breaking infix interpretation.
This prevents that by skipping leftmost-parenthesized wrapping recursion for operators that do not require surrounding whitespace.