-
Notifications
You must be signed in to change notification settings - Fork 224
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
Only print ?
for Scala 2 dialect if explicitly used
#2759
Conversation
scalameta/trees/shared/src/main/scala/scala/meta/internal/prettyprinters/TreeSyntax.scala
Outdated
Show resolved
Hide resolved
scalameta/trees/shared/src/main/scala/scala/meta/internal/prettyprinters/TreeSyntax.scala
Outdated
Show resolved
Hide resolved
scalameta/trees/shared/src/main/scala/scala/meta/internal/prettyprinters/TreeSyntax.scala
Outdated
Show resolved
Hide resolved
scalameta/trees/shared/src/main/scala/scala/meta/internal/prettyprinters/TreeSyntax.scala
Show resolved
Hide resolved
tests/jvm/src/test/scala/scala/meta/tests/prettyprinters/RegressionSyntaxSuite.scala
Show resolved
Hide resolved
1058513
to
37cadbc
Compare
scalameta/trees/shared/src/main/scala/scala/meta/internal/prettyprinters/TreeSyntax.scala
Show resolved
Hide resolved
37cadbc
to
b022d64
Compare
tests/jvm/src/test/scala/scala/meta/tests/prettyprinters/RegressionSyntaxSuite.scala
Outdated
Show resolved
Hide resolved
cc41aa3
to
529281b
Compare
check("new-dialect-scala213", "type T = List[_]", "type T = List[_]", newDialect = Some(dialects.Scala213))( | ||
dialects.Scala211 | ||
) | ||
check("new-dialect-scala213-from-scala3", "type T = List[?]", "type T = List[?]", newDialect = Some(dialects.Scala213))( |
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.
i give up :) i am very bad at reviewing today. very sorry. please merge whenever you're ready.
i think i meant this test (original was underscore, not ?
):
check("override-underscore-scala3-to-scala213", "type T = List[_]", "type T = List[?]")(dialects.Scala3)
it could also be
check("override-underscore-scala212-to-scala213", "type T = List[_]", "type T = List[?]")(dialects.Scala212)
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.
The two examples will not work since the original code has _
, so when printing in 212 or 213 we will not introduce ?
. I think it's a correct logic in this case.
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.
got it, thank you. approved earlier.
Previously, we would always print `?` for the placeholder even if the Scala version wouldn't allow it. Now, we only print `?` if it was explicitely used in the code. Fixes scalameta#2751 The other solution would be introducing another dialect, but that would require a lot of people having to also change that, so it would require a lot more effort in the long run.
529281b
to
8407933
Compare
Confirmed this fixed my problem! Awesome work, really appreciate it! |
Great to hear! |
Previously, we would always print
?
for the placeholder even if the Scala version wouldn't allow it. Now, we only print?
if it was explicitely used in the code.Fixes #2751
The other solution would be introducing another dialect, but that would require a lot of people having to also change that, so it would require a lot more effort in the long run.