-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Consistent naming of output-version settings #14606
Conversation
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 don't think renaming Xtarget is worth it, it's an advanced option anyway and the name as is makes it clearer it matches the semantics of -target in java. Otherwise LGTM.
@@ -103,7 +103,9 @@ trait CommonScalaSettings: | |||
val pageWidth: Setting[Int] = IntSetting("-pagewidth", "Set page width", ScalaSettings.defaultPageWidth, aliases = List("--page-width")) | |||
val silentWarnings: Setting[Boolean] = BooleanSetting("-nowarn", "Silence all warnings.", aliases = List("--no-warnings")) | |||
|
|||
val release: Setting[String] = ChoiceSetting("-release", "release", "Compile code with classes specific to the given version of the Java platform available on the classpath and emit bytecode for this version.", ScalaSettings.supportedReleaseVersions, "", aliases = List("--release")) | |||
val javaOutputVersion: Setting[String] = ChoiceSetting("-java-output-version", "version", "Compile code with classes specific to the given version of the Java platform available on the classpath and emit bytecode for this version. Corresponds to -release flag in javac.", ScalaSettings.supportedReleaseVersions, "", aliases = List("-release", "--release")) |
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.
Could you make a PR to Scala 2 to also rename -release there?
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'm not sure we should rename it for scala 2 as the semantics of -release and -target are slightly different there #8634 (comment)
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.
Yeah I think that would also require changing the scala 2 flag to also set the target, IMO the current behavior is an oversight.
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.
without over-promising, maybe I'll take the opportunity to understand the issues and ergonomics.
Regarding -Xtarget I think it's good to keep symmetry anyway. |
Following on from #9982 and discussion in https://github.com/scala/scala/pull/9982/files#r1434172490, this change enables `-java-output-version` as an alternative name for the `-release` flag in Scala 2, as in Scala 3 `-java-output-version` is the preferred flag name (see scala/scala3#14606 ), with the alias of `-release` still permitted. Correspondingly, `-Xunchecked-java-output-version` is also included as an alternative for `-target`, though note use of this flag is discouraged in favour of `-java-output-version`/`-release` for Java 9 and above.
Following the scheme https://contributors.scala-lang.org/t/improving-scala-3-forward-compatibility/5298/22
rename compiler settings:
-Yscala-release -> -scala-output-version
-release -> -java-output-version
-Xtarget -> -Xunchecked-java-output-version
(leaving old names as aliases for compatibility)