-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Handle help, version and @file parameters in scalac and scaladoc #11476
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
Conversation
ce13b5e
to
8603bac
Compare
@BarkingBad For comparison here is what user@host W:\dotty
$ echo %KOTLIN_HOME%
C:\opt\kotlinc-1.4.30
user@host W:\dotty
$ %KOTLIN_HOME%\bin\kotlinc.bat -help 2>&1 | tail -8
-nowarn Generate no warnings
-verbose Enable verbose logging output
-version Display compiler version
@<argfile> Read compiler arguments and file paths from the given file
Note: on Windows, arguments that contain delimiter characters (whitespace, =, ;, ,) need to be surrounded with double quotes (").
For details, see https://kotl.in/cli |
Help issue: aratajczak@aratajczak:~/Dokumenty/dotty$ ./bin/scalac -help 2>&1 | head -4
Usage: scalac <options> <source files>
where possible standard options include:
-P Pass an option to a plugin, e.g. -P:<plugin>:<opt>
-X Print a synopsis of advanced options.
aratajczak@aratajczak:~/Dokumenty/dotty$ ./bin/scala -help 2>&1 | head -4
Usage: scala <options> <source files>
where possible standard options include:
-P Pass an option to a plugin, e.g. -P:<plugin>:<opt>
-X Print a synopsis of advanced options.
aratajczak@aratajczak:~/Dokumenty/dotty$ ./bin/scaladoc -help 2>&1 | head -4
Usage: scaladoc <options> <source files>
where possible standard options include:
-P Pass an option to a plugin, e.g. -P:<plugin>:<opt>
-bootclasspath Override location of bootstrap class files. Version issue: aratajczak@aratajczak:~/Dokumenty/dotty$ ./bin/scalac -version
Scala compiler version 3.0.0-RC2-bin-SNAPSHOT-git-cd0a6da -- Copyright 2002-2021, LAMP/EPFL
aratajczak@aratajczak:~/Dokumenty/dotty$ ./bin/scala -version
Scala code runner version 3.0.0-RC2-bin-SNAPSHOT-git-cd0a6da -- Copyright 2002-2021, LAMP/EPFL
aratajczak@aratajczak:~/Dokumenty/dotty$ ./bin/scaladoc -version
Scaladoc version 3.0.0-RC2-bin-SNAPSHOT-git-cd0a6da -- Copyright 2002-2021, LAMP/EPFL @files issue:
@michelou could you check just in case that |
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.
Scaladoc related changes look ok
@BarkingBad Many thanks ! Ouputs for MinGW environment
user@host MINGW64 /w
$ echo $JAVA_HOME
/c/opt/jdk-11.0.10+9/
user@host MINGW64 /w
$ /c/opt/scala-3.0.0-RC2-bin-SNAPSHOT/bin/scala -version
Scala code runner version 3.0.0-RC2-bin-SNAPSHOT-git-ce1e2a2 -- Copyright 2002-2021, LAMP/EPFL
user@host MINGW64 /w
$ /c/opt/scala-3.0.0-RC2-bin-SNAPSHOT/bin/scalac -version
Scala compiler version 3.0.0-RC2-bin-SNAPSHOT-git-ce1e2a2 -- Copyright 2002-2021, LAMP/EPFL
user@host MINGW64 /w
$ /c/opt/scala-3.0.0-RC2-bin-SNAPSHOT/bin/scaladoc -version
Scaladoc version 3.0.0-RC2-bin-SNAPSHOT-git-ce1e2a2 -- Copyright 2002-2021, LAMP/EPFL
user@host MINGW64 /w
$ /c/opt/scala-3.0.0-RC2-bin-SNAPSHOT/bin/scala -help 2>&1 | head -4
Usage: scala <options> <source files>
where possible standard options include:
-P Pass an option to a plugin, e.g. -P:<plugin>:<opt>
-X Print a synopsis of advanced options.
user@host MINGW64 /w
$ /c/opt/scala-3.0.0-RC2-bin-SNAPSHOT/bin/scalac -help 2>&1 | head -4
Usage: scalac <options> <source files>
where possible standard options include:
-P Pass an option to a plugin, e.g. -P:<plugin>:<opt>
-X Print a synopsis of advanced options.
user@host MINGW64 /w
$ /c/opt/scala-3.0.0-RC2-bin-SNAPSHOT/bin/scaladoc -help 2>&1 | head -4
Usage: scaladoc <options> <source files>
where possible standard options include:
-P Pass an option to a plugin, e.g. -P:<plugin>:<opt>
-bootclasspath Override location of bootstrap class files.
user@host MINGW64 /w/dotty
$ ls /c/temp/docs
user@host MINGW64 /w/dotty
$ cat scaladoc_opts?.txt
-d c:\\temp\\docs c:\\temp\\classes\\HelloWorld.tasty
-project HelloWorld
user@host MINGW64 /w/dotty
$ /c/opt/scala-3.0.0-RC2-bin-SNAPSHOT/bin/scaladoc @scaladoc_opts1.txt @scaladoc_opts2.txt
user@host MINGW64 /w/dotty
$ ls /c/temp/docs /c/temp/docs/api /c/temp/docs/api/_empty_
/c/temp/docs:
api favicon.ico fonts hljs images scaladoc.version scripts styles
/c/temp/docs/api:
_empty_ index.html
/c/temp/docs/api/_empty_:
'HelloWorld$.html' Windows environment
user@host W:\dotty
$ echo %JAVA_HOME%
C:\opt\jdk-11.0.10+9
user@host W:\dotty
$ c:\opt\scala-3.0.0-RC2-bin-SNAPSHOT\bin\scala -version
Scala code runner version 3.0.0-RC2-bin-SNAPSHOT-git-ce1e2a2 -- Copyright 2002-2021, LAMP/EPFL
user@host W:\dotty
$ c:\opt\scala-3.0.0-RC2-bin-SNAPSHOT\bin\scalac -version
Scala compiler version 3.0.0-RC2-bin-SNAPSHOT-git-ce1e2a2 -- Copyright 2002-2021, LAMP/EPFL
user@host W:\dotty
$ c:\opt\scala-3.0.0-RC2-bin-SNAPSHOT\bin\scaladoc -version
Scaladoc version 3.0.0-RC2-bin-SNAPSHOT-git-ce1e2a2 -- Copyright 2002-2021, LAMP/EPFL
user@host W:\dotty
$ ls c:\temp\docs
user@host W:\dotty
$ cat scaladoc_opts?.txt
-d c:\\temp\\docs c:\\temp\\classes\\HelloWorld.tasty
-project HelloWorld
user@host W:\dotty
$ c:\opt\scala-3.0.0-RC2-bin-SNAPSHOT\bin\scaladoc @scaladoc_opts1.txt @scaladoc_opts2.txt
user@host W:\dotty
$ ls c:\temp\docs c:\temp\docs\api c:\temp\docs\api\_empty_
'c:\temp\docs':
api favicon.ico fonts hljs images scaladoc.version scripts styles
'c:\temp\docs\api':
_empty_ index.html
'c:\temp\docs\api\_empty_':
'HelloWorld$.html' |
cd0a6da
to
7e69e2d
Compare
@@ -47,7 +47,7 @@ class SettingsTests { | |||
|
|||
inContext { | |||
val args = List("-foo", "b", "-bar", "1") | |||
val summary = Settings.processArguments(args, true) | |||
val summary = Settings.processArguments(args, summon[Context].settingsState, true) |
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.
You can use ctx
instead of summon[Context]
if you import Contexts._
which contains inline def ctx(using ctx: Context): Context = ctx
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.
Actually, it was all wrong, because we used Seq of arguments from ScalaSettings
, not our custom Settings
. It was working well, becuase we were inserting values on positions 0 and 1, then we were retrieving them. Nevertheless, we have to use Settings.defaultState
here, but we can actually make it default value, since most of the time we won't have semi initialized state.
val files = fileNames.map(ctx.getFile) | ||
(files, fromTastySetup(files)) | ||
val fileNamesOrNone = command.checkUsage(summary, sourcesRequired)(using ctx.settings)(using ctx.settingsState) | ||
fileNamesOrNone.fold((None, ictx)) { fileNames => |
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 very important but we rarely use fold on Option in the compiler, instead we prefer to match on Some(...) and None
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.
Fill fix
if (line.startsWith(quit)) ctx.reporter | ||
else loop(line split "\\s+", nextCtx) | ||
var (possibleFiles, ctx) = setup(args, prevCtx) | ||
if possibleFiles.isDefined then |
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 would pattern match on possibleFiles here instead of using isDefined/get
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.
Will fix
override def versionMsg: String = s"Scala compiler $versionString -- $copyrightString" | ||
override def ifErrorsMsg: String = " scalac -help gives more information" | ||
abstract class CompilerCommand extends CliCommand: | ||
final type ConcreteSettings = ScalaSettings |
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 final on a type alias does anything.
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.
Can remove
def setup(args: Array[String], rootCtx: Context): (List[AbstractFile], Context) = { | ||
protected def command: CompilerCommand = ScalacCommand | ||
|
||
def setup(args: Array[String], rootCtx: Context): (Option[List[AbstractFile]], Context) = { |
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.
An Option of a List is weird, the documentation of this method should explain clearly how None differs from Some(Nil) 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.
Will update documentation
7e69e2d
to
b79bc84
Compare
@BarkingBad Java behavioruser@host MINGW64 /w/dotty
$ echo $JAVA_HOME
/c/opt/jdk-11.0.10+9/
user@host MINGW64 /w/dotty
$ /c/opt/jdk-11.0.10+9/bin/javac @dummy.txt
error: file not found: dummy.txt Scala 3 behavioruser@host MINGW64 /w/dotty
$ /c/opt/scala-3.0.0-RC2-bin-SNAPSHOT/bin/scaladoc @dummy.txt
java.io.FileNotFoundException: argument file dummy.txt could not be found
at dotty.tools.dotc.config.CliCommand.expandArg$1(CliCommand.scala:52)
at dotty.tools.dotc.config.CliCommand.expandedArguments$2$$anonfun$1(CliCommand.scala:61)
at scala.collection.immutable.List.flatMap(List.scala:293)
at dotty.tools.dotc.config.CliCommand.expandedArguments$1(CliCommand.scala:62)
at dotty.tools.dotc.config.CliCommand.distill(CliCommand.scala:65)
at dotty.tools.dotc.config.CliCommand.distill$(CliCommand.scala:12)
at dotty.tools.scaladoc.ScaladocCommand$.distill(ScaladocCommand.scala:19)
at dotty.tools.scaladoc.Scaladoc$.extract(Scaladoc.scala:79)
at dotty.tools.scaladoc.Scaladoc$.run(Scaladoc.scala:49)
at dotty.tools.scaladoc.Main.run(Main.scala:18)
at dotty.tools.scaladoc.Main$.main(Main.scala:24)
at dotty.tools.scaladoc.Main.main(Main.scala)
user@host MINGW64 /w/dotty
$ /c/opt/scala-3.0.0-RC2-bin-SNAPSHOT/bin/scalac @dummy.txt
Exception in thread "main" java.io.FileNotFoundException: argument file dummy.txt could not be found
at dotty.tools.dotc.config.CliCommand.expandArg$1(CliCommand.scala:52)
at dotty.tools.dotc.config.CliCommand.expandedArguments$2$$anonfun$1(CliCommand.scala:61)
at scala.collection.immutable.List.flatMap(List.scala:293)
at dotty.tools.dotc.config.CliCommand.expandedArguments$1(CliCommand.scala:62)
at dotty.tools.dotc.config.CliCommand.distill(CliCommand.scala:65)
at dotty.tools.dotc.config.CliCommand.distill$(CliCommand.scala:12)
at dotty.tools.dotc.config.CompilerCommand.distill(CompilerCommand.scala:12)
at dotty.tools.dotc.Driver.setup(Driver.scala:79)
at dotty.tools.dotc.Driver.process(Driver.scala:199)
at dotty.tools.dotc.Driver.process(Driver.scala:169)
at dotty.tools.dotc.Driver.process(Driver.scala:181)
at dotty.tools.dotc.Driver.main(Driver.scala:210)
at dotty.tools.dotc.Main.main(Main.scala) Scala 2 behavioruser@host MINGW64 /w/dotty
$ /c/opt/scala-2.13.5/bin/scaladoc @dummy.txt
Exception in thread "main" java.io.FileNotFoundException: argument file dummy.txt could not be found
at scala.tools.nsc.CompilerCommand.expandArg(CompilerCommand.scala:132)
at scala.tools.nsc.CompilerCommand.$anonfun$processArguments$1(CompilerCommand.scala:142)
at scala.tools.nsc.CompilerCommand.processArguments(CompilerCommand.scala:141)
at scala.tools.nsc.CompilerCommand.<init>(CompilerCommand.scala:23)
at scala.tools.nsc.ScalaDoc$Command.<init>(ScalaDoc.scala:89)
at scala.tools.nsc.ScalaDoc.process(ScalaDoc.scala:32)
at scala.tools.nsc.ScalaDoc$.main(ScalaDoc.scala:99)
at scala.tools.nsc.ScalaDoc.main(ScalaDoc.scala)
user@host MINGW64 /w/dotty
$ /c/opt/scala-2.13.5/bin/scalac @dummy.txt
Exception in thread "main" java.io.FileNotFoundException: argument file dummy.txt could not be found
at scala.tools.nsc.CompilerCommand.expandArg(CompilerCommand.scala:132)
at scala.tools.nsc.CompilerCommand.$anonfun$processArguments$1(CompilerCommand.scala:142)
at scala.tools.nsc.CompilerCommand.processArguments(CompilerCommand.scala:141)
at scala.tools.nsc.CompilerCommand.<init>(CompilerCommand.scala:23)
at scala.tools.nsc.Driver.process(Driver.scala:56)
at scala.tools.nsc.Driver.main(Driver.scala:82)
at scala.tools.nsc.Main.main(Main.scala) |
* would change the default behaviour of the compiler. | ||
* | ||
* @return A tuple of Option of List and Context. | ||
* If there is no setting preventing us from continuing compilation, function returns Some(List) |
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 a method, not a function:
* If there is no setting preventing us from continuing compilation, function returns Some(List) | |
* If there is no setting like `-help` preventing us from continuing compilation, this method returns Some(...) |
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.
Will fix
* @return A tuple of Option of List and Context. | ||
* If there is no setting preventing us from continuing compilation, function returns Some(List) | ||
* and Context updated respectively with compilation files. In particular, it can be Some(Nil). | ||
* If compilation should be interruped, function returns None and an old Context. |
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.
* If compilation should be interruped, function returns None and an old Context. | |
* If compilation should be interrupted, this method returns None and an old Context. |
But if you're returning the original Context, then there's no point in returning it at all and you could change the result type of this method to Option[(List[AbstractFile], Context)]
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.
Good point, will change
e5a54cd
to
56d3e56
Compare
@michelou I have changed the error handling, instead of throwing an exception I used context reporter to report an error, however, it does not break the execution by default (with no fatal warnings etc.) and continue as if the file was empty. Should I change it to always stop if there is problem with file, or an error is sufficient? |
/** Setup context with initialized settings from CLI arguments, then check if there are any settings that | ||
* would change the default behaviour of the compiler. | ||
* | ||
* @return An Option of tuple of List and Context. |
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.
That line isn't really necessary since it just repeats the type without adding information.
* | ||
* @return An Option of tuple of List and Context. | ||
* If there is no setting like `-help` preventing us from continuing compilation, this method returns | ||
* Some of files to compile and updated Context. |
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.
* Some of files to compile and updated Context. | |
* a list of files to compile and an updated Context. |
@@ -19,7 +19,9 @@ class IDEDecompilerDriver(val settings: List[String]) extends dotc.Driver { | |||
val rootCtx = initCtx.fresh.addMode(Mode.Interactive | Mode.ReadPositions | Mode.ReadComments) | |||
rootCtx.setSetting(rootCtx.settings.YretainTrees, true) | |||
rootCtx.setSetting(rootCtx.settings.fromTasty, true) | |||
val ctx = setup(settings.toArray :+ "dummy.scala", rootCtx)._2 | |||
val ctx = setup(settings.toArray :+ "dummy.scala", rootCtx) match |
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.
We should never be passing --help
here or another flag that interrupts compilation so you could just do .get
on the result of setup
.
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.
Will change
@@ -33,7 +33,10 @@ private class QuoteDriver(appClassloader: ClassLoader) extends Driver: | |||
new VirtualDirectory("<quote compilation output>") | |||
end outDir | |||
|
|||
val (_, ctx0: Context) = setup(settings.compilerArgs.toArray :+ "dummy.scala", initCtx.fresh) | |||
val freshContext = initCtx.fresh | |||
val ctx0 = setup(settings.compilerArgs.toArray :+ "dummy.scala", freshContext) match |
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.
Same comment as in IDEDecompilingDriver
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.
Will change
* Possible reason for unsuccessful run are raised flags in CLI like --help or --version | ||
*/ | ||
final def tryRunning = if shouldStart then | ||
println("Starting scala3 REPL...") |
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 see this was here before, but I would just delete that line, the repl already prints enough stuff when it starts.
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.
Can remove
56d3e56
to
c4c63fc
Compare
@BarkingBad Here's what I get now (after your force-pushed). The error message is followed by the help message (that's ok for me since my priority was to get rid of user@host MINGW64 /w/dotty
$ /c/opt/scala-3.0.0-RC2-bin-SNAPSHOT/bin/scaladoc @dummy.txt 2>&1 | head -8
Argument file dummy.txt could not be found
Usage: scaladoc <options> <source files>
where possible standard options include:
-P Pass an option to a plugin, e.g. -P:<plugin>:<opt>
-bootclasspath Override location of bootstrap class files.
-classpath Specify where to find user class files.
Default: ..
-color Colored output
user@host MINGW64 /w/dotty
$ /c/opt/scala-3.0.0-RC2-bin-SNAPSHOT/bin/scaladoc @dummy.txt 2>&1 | head -7
Argument file dummy.txt could not be found
Usage: scaladoc <options> <source files>
where possible standard options include:
-P Pass an option to a plugin, e.g. -P:<plugin>:<opt>
-bootclasspath Override location of bootstrap class files.
-classpath Specify where to find user class files.
Default: .. |
The help message pops up because incorrect path makes @file argument transparent, and since there is no other parameter provided we assume it was run with no parameters and show help message. I can fix it if it is still a concern |
Draft preview of changes for Arguments parser to decouple it from Dotty Context and fix three raised issues.
I know that some things can be written simplier, I will fix these on Monday morning, just want to leave it as a draft to know if these changes are generally OK.