Scalac preset option in directive #3615
Conversation
There was a problem hiding this comment.
Note: we should support this both via a directive and as a command line option (similarly to how toolkits are done).
Then, the command line should be able to override what we have in directives (be careful not to concatenate the options, if a different preset is chosen from the command line than in a directive.
Finally, we'll need to test if duplicates are handled correctly (for example, if a user passes an option explicitly, but also uses a preset which contains it, we need to handle the duplicates).
This means quite a bunch test scenarios to write.
| case Some(other) => | ||
| List(Left( | ||
| InputsException( | ||
| s"Unknown preset options: $other. Available options are: suggested, ci, strict" |
There was a problem hiding this comment.
We should have an enumeration of the available presets somewhere, with tied option lists.
then we should just call the full list here, rather than hardcoding it like this.
There was a problem hiding this comment.
Yeah, where should this be defined. I was only making a quick draft just to see if I understood the requirement correctly. The issue seems to be really interesting as I tend to add some options manually and I never remember what should be used.
There was a problem hiding this comment.
I added these options as enums
| @@ -62,4 +92,9 @@ object ScalacOptions { | |||
| ) | |||
| ) | |||
| } | |||
|
|
|||
| // todo: this should be based on the scala version. This might not be the correct location | |||
There was a problem hiding this comment.
yup... might be annoying to keep up-to-date as well.
I actually wonder if the best place for defining the presets wouldn't be the compiler itself... but that could be done as an improvement.
just theorising, since this is an early draft.
There was a problem hiding this comment.
the 3 presets for a start are good I think (having suggested, ci and strict; still need to establish what would each of them entail)
There was a problem hiding this comment.
Yeah, I just wanted to add some ideas, I am also not fully clear what should they be, but added there just to make sure that we should handle multiple presets.
There was a problem hiding this comment.
Regarding what they should be, I believe the idea was to start with something like what https://github.com/typelevel/sbt-tpolecat has...
We could also start a discussion for suggestions somewhere... A thread on https://contributors.scala-lang.org/, maybe... or just a discussion on this repo?
Either way, if/when suggestions on which options should be included come, we could tweak it gradually.
The feature should first come out as experimental, anyway.
There was a problem hiding this comment.
I thought about the duplicates yday, but I saw some test related to duplicate scalaC args are ignored or something like that. SO thought it can be ignored. I will try to handle them. How about getting the scalaVersion? We should have the sugested and other presets based on the scala versions right. I assumes optsForScala212, optsForScala213, optsForScala3 which supports all versions within 2.13, 2.12 and 3.
Can you point me how to get tht scala version within this scalacOptions code?
There was a problem hiding this comment.
If the command line args needs to be processed, then is it not possible to add the scalac options in the ScalacOptions.scala file right? I mean, tif the preset is also passed as command line option, we need to keep the preset value till the last moment and then decide priority (command line over directive). So, the preset value should be somehow carried over and not the flattened scalac options). Am I right?
There was a problem hiding this comment.
@Gedochao Could you have a look at it when you get time?
There was a problem hiding this comment.
Can you point me how to get tht scala version within this scalacOptions code?
You can't. We don't know what's the final Scala version until using directives are parsed.
If the command line args needs to be processed, then is it not possible to add the scalac options in the ScalacOptions.scala file right? I mean, tif the preset is also passed as command line option, we need to keep the preset value till the last moment and then decide priority (command line over directive). So, the preset value should be somehow carried over and not the flattened scalac options). Am I right?
Yep, the presets can only be applied once the Scala version is finalised, which generally should be after (or within) scala.build.CrossSources.forInputs.
I thought about the duplicates yday, but I saw some test related to duplicate scalaC args are ignored or something like that.
We do have handling for when there's duplicates introduced by the command line vs directives. If duplicates are explicitly added by the user in one context, we don't remove them and let the compiler handle it.
However, for when a preset has a duplicate with whatever explicit options the user passed, you need to make sure it's handled correctly. Just add a test for this case.
Implementation for #3596