-
Notifications
You must be signed in to change notification settings - Fork 3.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
SI-9154 scalac offers extra help on bad option #4336
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,8 @@ abstract class Driver { | |
protected var settings: Settings = _ | ||
|
||
protected def scalacError(msg: String): Unit = { | ||
reporter.error(FakePos("scalac"), msg + "\n scalac -help gives more information") | ||
val indent = "\u0020" * 2 | ||
reporter.error(FakePos("scalac"), f"${msg}%n${indent}`scalac -help` gives more information") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fancy! Though maybe more so than necessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right as usual. I'll change it to:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean, I know you got f-interpolation on your mind and all, but... just saying. |
||
} | ||
|
||
protected def processSettingsHook(): Boolean = { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,9 +70,23 @@ class MutableSettings(val errorFn: String => Unit) | |
case "" :: xs => | ||
loop(xs, residualArgs) | ||
case x :: xs => | ||
def error() = { | ||
val err = s"bad option: '$x'" | ||
// see if there's an option that looks like it | ||
val suffix = x.stripPrefix("-X").stripPrefix("-Y").stripPrefix("-") | ||
val leaders = List("-", "-X", "-Y") | ||
val allprefixes = leaders map (_ + suffix) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why add back the leaders? It just restricts the match to cases where the key word was at the beginning. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thx, that was because my first thought was only to show prefixes. After I'd moved on to my second thought, it worked for anything after hyphen, so java yielded -Ygen-javap etc. |
||
val indent = "\u0020" * 2 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, not really a fan of this pattern. |
||
val helping = help || Xhelp || Yhelp | ||
|
||
def similar(s: Setting) = !s.isDeprecated && (allprefixes exists (s.name contains _)) | ||
val alts = (allSettings filter similar map (_.name)).toList.sorted mkString f"%n$indent" | ||
val msg = if (helping && alts.nonEmpty) f"$err%n${indent}Similar options:%n$indent$alts" else err | ||
errorFn(msg) | ||
} | ||
if (x startsWith "-") { | ||
parseParams(args) match { | ||
case newArgs if newArgs eq args => errorFn(s"bad option: '$x'") ; (false, args) | ||
case newArgs if newArgs eq args => error() ; (false, args) | ||
case newArgs => loop(newArgs, residualArgs) | ||
} | ||
} | ||
|
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 superfluous unless you intend some tab-like behavior with indentation (which isn't here). Just stick two spaces in in place of
${indent}
below?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'll use a formatting width as in my reply to adriaan. Two spaces is too hard to verify at a glance, even in monospace.
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.
BTW, yes I got carried away.