From a93d9e77adb0d388c458dbcae2d582609d9b58f1 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Tue, 16 Jul 2019 13:16:03 -0700 Subject: [PATCH] Relax strict commands The recent changes to make the multi parser strict broke any multi command, or alias, where the multi command contained a command or task that was not yet defined, but was possibly added by reload. This was reported as #4869. I had had to work around this issue in ScriptedTests by running `reload` and `setUpScripted` separately instead of as a multi command. This workaround doesn't work for aliasing boot, which has been a recommended approach by Mark Harrah since 2011. To fix this, I relax the strict parser. We don't require that the parser be valid to create a multi command string. In the multiApplied state transformation, however, we validate all of the commands up to 'reload'. Since there is no way to validate any commands to the right of 'reload, we optimistically allow those commands to run. So long as there is no 'reload' in the multi commands, all of the commands will be validated. --- .../src/main/scala/sbt/BasicCommands.scala | 19 ++++++++++++++----- .../sbt/scriptedtest/ScriptedTests.scala | 18 ++++++++---------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/main-command/src/main/scala/sbt/BasicCommands.scala b/main-command/src/main/scala/sbt/BasicCommands.scala index cdcbd32c1..b6d68c685 100644 --- a/main-command/src/main/scala/sbt/BasicCommands.scala +++ b/main-command/src/main/scala/sbt/BasicCommands.scala @@ -158,11 +158,12 @@ object BasicCommands { val semi = token(OptSpace ~> ';' ~> OptSpace) val nonDelim = charClass(c => c != '"' && c != '{' && c != '}', label = "not '\"', '{', '}'") val components = ((nonSemi & nonDelim) | StringEscapable | braces('{', '}')).+ - val cmdPart = matched(components) + val cmdPart = matched(components).examples() val completionParser: Option[Parser[String]] = state.map(s => OptSpace ~> matched(s.nonMultiParser) <~ OptSpace) - val cmdParser = completionParser.map(sp => sp & cmdPart).getOrElse(cmdPart).map(_.trim) + val cmdParser = + completionParser.map(sp => (sp & cmdPart) | cmdPart).getOrElse(cmdPart).map(_.trim) val multiCmdParser: Parser[String] = semi ~> cmdParser /* * There are two cases that need to be handled separately: @@ -187,7 +188,14 @@ object BasicCommands { def multiParser(s: State): Parser[List[String]] = multiParserImpl(Some(s)) - def multiApplied(state: State): Parser[() => State] = + def multiApplied(state: State): Parser[() => State] = { + def generateCommands(commands: List[String]): State = + commands + .takeWhile(_ != "reload") + .collectFirst { + case c if Parser.parse(c, state.combinedParser).isLeft => c :: state + } + .getOrElse(commands ::: state) Command.applyEffect(multiParserImpl(Some(state))) { // the (@ _ :: _) ensures tail length >= 1. case commands @ first :: (tail @ _ :: _) => @@ -230,10 +238,11 @@ object BasicCommands { } }.headOption match { case Some(s) => s() - case _ => commands ::: state + case _ => generateCommands(commands) } - case commands => commands ::: state + case commands => generateCommands(commands) } + } val multi: Command = Command.custom(multiApplied, Help(Multi, MultiBrief, MultiDetailed), Multi) diff --git a/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala b/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala index c92e29e12..c5080006c 100644 --- a/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala +++ b/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala @@ -328,20 +328,18 @@ final class ScriptedTests( IO.write(tempTestDir / "project" / "InstrumentScripted.scala", pluginImplementation) def sbtHandlerError = sys error "Missing sbt handler. Scripted is misconfigured." val sbtHandler = handlers.getOrElse('>', sbtHandlerError) + val commandsToRun = ";reload;setUpScripted" + val statement = Statement(commandsToRun, Nil, successExpected = true, line = -1) // Run reload inside the hook to reuse error handling for pending tests val wrapHook = (file: File) => { preHook(file) - Seq("reload", "setUpScripted") - .map(Statement(_, Nil, successExpected = true, line = -1)) - .foreach { statement => - try runner.processStatement(sbtHandler, statement, states) - catch { - case t: Throwable => - val newMsg = "Reload for scripted batch execution failed." - throw new TestException(statement, newMsg, t) - } - } + try runner.processStatement(sbtHandler, statement, states) + catch { + case t: Throwable => + val newMsg = "Reload for scripted batch execution failed." + throw new TestException(statement, newMsg, t) + } } commonRunTest(label, tempTestDir, wrapHook, handlers, runner, states, buffer)