-
Notifications
You must be signed in to change notification settings - Fork 277
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
Scalameta: upgrade to v4.5.1 #3164
Conversation
A66 /* c1 */ | /* c2 */ A67 & A68 | | ||
A69 & A70[T70] | ||
type T = | ||
A1 & A2 | A3 & A4 | A5 & A6 /* c1 */ | /* c2 */ A7 & A8 | A9 & A10[ |
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.
while this might look bad, in reality it's because in the original code snippet, there were no breaks between infix types. for any existing code which had previously been formatted using source=keep and Type.Or trees, the modified code wouldn't change formatting because it would still preserve breaks.
A68 | | ||
A69 & | ||
A70[T70] | ||
A1 & A2 | A3 & A4 | A5 & A6 | |
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.
Shouldn't unfold try to break it to separate lines as previously? This looks ok, but I am not sure about the expected bahaviour in case of long infix types
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.
no, unfold
uses afterInfix=many
which breaks a little more often than fold
s afterInfix=some
but not on every infix.
perhaps it could be improved but that would introduce changes to existing code which used {Term,Type}.ApplyInfix
in other situations, not just Type.{And,Or}
...
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.
It's fine, I was just wondering
val a: FirstTypeInTheInterSectionType & | ||
SecondTypeInTheInterSectionType | ||
def hello() | ||
: FirstTypeInTheInterSectionType & SecondTypeInTheInterSectionType & ThirdTypeInTheInterSectionType = { |
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.
This seems to now exceed 30 column limit, is this correct?
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.
correct, but the configuration uses keep
here.
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.
wasn't keep supposed to still break lines if it's too long?
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.
not for infix. infix formatting rules were added about a year ago, they are somewhat complex, and before that the formatter always kept breaks around infix operators as is, regardless of line length.
Also, remove handling of deprecated Type.{And,Or} trees and instead switch scala3 type infixes to regular infix handling.
Also, remove handling of deprecated Type.{And,Or} trees and instead switch scala3 type infixes to regular infix handling. Fixes #3153.