Skip to content
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

Add support for reading command-line options from file(s) (#191) #317

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ object CliAssertion {
)(implicit cliConfig: CliConfig): ZIO[R, Throwable, TestResult] =
check(pairs) { case CliRepr(params, assertion) =>
command
.parse(params, cliConfig)
.parse(params, Nil, cliConfig)
.map(Right(_))
.catchAll { case e: ValidationError =>
ZIO.succeed(Left(e))
Expand All @@ -110,7 +110,7 @@ object CliAssertion {
)(implicit cliConfig: CliConfig): ZIO[R, Throwable, TestResult] =
check(pairs) { case CliRepr(params, assertion) =>
command
.parse(params, cliConfig)
.parse(params, Nil, cliConfig)
.map(TestReturn.convert)
.map(Right(_))
.catchSome { case e: ValidationError =>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package zio.cli

trait FileArgsPlatformSpecific {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be private[cli].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean

private[cli] trait FileArgsPlatformSpecific {

or

trait FileArgsPlatformSpecific private[cli] {

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did the first one, looked at other *PlatformSpecifc

val default: FileArgs = FileArgs.Noop
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package zio.cli

trait FileArgsPlatformSpecific {
val default: FileArgs = FileArgs.Live
}
82 changes: 82 additions & 0 deletions zio-cli/jvm/src/test/scala/zio/cli/LiveFileArgsSpec.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package zio.cli

import zio._
import zio.internal.stacktracer.SourceLocation
import zio.test._

import java.nio.file.{Files, Path}

object LiveFileArgsSpec extends ZIOSpecDefault {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be put also in the native folder, right?

Copy link
Contributor Author

@Kalin-Rudnicki Kalin-Rudnicki Jun 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can add this, yes.
I am really annoyed... Intellij creates multiple cross-platform directories:

  • js-jvm
  • js-native
  • jvm-native
    But sbt doesnt seem to correctly add these dependencies as I would expect.
    I tried originally adding this spec in jvm-native, but it didnt seem to realize anything in shared existed :/

private val createTempDirectory: RIO[Scope, Path] =
for {
random <- Random.nextUUID
path <-
ZIO.attempt(Files.createTempDirectory(random.toString)).withFinalizer(f => ZIO.attempt(f.toFile.delete()).orDie)
} yield path

private def resolvePath(path: Path, paths: List[String]): Path =
if (paths.nonEmpty) path.resolve(paths.mkString("/"))
else path

private def makeTest(name: String)(cwd: List[String], home: List[String])(
writeFiles: (List[String], String)*
)(
exp: (List[String], List[String])*
)(implicit loc: SourceLocation): Spec[Scope, Throwable] =
test(name) {
for {
// setup
dir <- createTempDirectory
_ <- TestSystem.putProperty("user.dir", resolvePath(dir, cwd).toString)
_ <- TestSystem.putProperty("user.home", resolvePath(dir, home).toString)
_ <- ZIO.foreachDiscard(writeFiles) { case (paths, contents) =>
val writePath = resolvePath(dir, paths :+ s".$cmd")
val parentFile = writePath.getParent.toFile
ZIO.attempt(parentFile.mkdirs()).unlessZIO(ZIO.attempt(parentFile.exists())) *>
ZIO.writeFile(writePath, contents)
}

// test
result <- FileArgs.Live.getArgsFromFile(cmd)
resolvedExp = exp.toList.map { case (paths, args) =>
FileArgs.ArgsFromFile(resolvePath(dir, paths :+ s".$cmd").toString, args)
}

} yield assertTrue(result == resolvedExp)
}

private val cmd: String = "command"

override def spec: Spec[TestEnvironment with Scope, Any] =
suite("FileArgsSpec")(
makeTest("empty")(List("abc", "home"), List("abc", "home"))()(),
makeTest("home in cwd parent path")(List("abc", "home", "d", "e", "f"), List("abc", "home"))(
List("abc", "home", "d") -> "d\nd\n\n",
List("abc", "home", "d", "e") -> "e\ne\n\n",
List("abc", "home", "d", "e", "f") -> "f\nf\n\n",
List("abc", "home") -> "_home_"
)(
List("abc", "home", "d", "e", "f") -> List("f", "f"),
List("abc", "home", "d", "e") -> List("e", "e"),
List("abc", "home", "d") -> List("d", "d"),
List("abc", "home") -> List("_home_") // only appears once
),
makeTest("home not in cwd parent path")(List("abc", "cwd", "d", "e", "f"), List("abc", "home"))(
List("abc", "cwd", "d") -> "d\nd\n\n",
List("abc", "cwd", "d", "e") -> "e\ne\n\n",
List("abc", "cwd", "d", "e", "f") -> "f\nf\n\n",
List("abc", "home") -> "_home_"
)(
List("abc", "cwd", "d", "e", "f") -> List("f", "f"),
List("abc", "cwd", "d", "e") -> List("e", "e"),
List("abc", "cwd", "d") -> List("d", "d"),
List("abc", "home") -> List("_home_")
),
makeTest("parent dirs of home are not searched")(Nil, List("abc", "home"))(
List("abc") -> "a\nb"
)(
)
) @@ TestAspect.withLiveRandom

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package zio.cli

trait FileArgsPlatformSpecific {
val default: FileArgs = FileArgs.Live
}
14 changes: 10 additions & 4 deletions zio-cli/shared/src/main/scala/zio/cli/CliApp.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ import scala.annotation.tailrec
*/
sealed trait CliApp[-R, +E, +A] { self =>

def run(args: List[String]): ZIO[R, CliError[E], Option[A]]
def runWithFileArgs(args: List[String]): ZIO[R & FileArgs, CliError[E], Option[A]]

final def run(args: List[String]): ZIO[R, CliError[E], Option[A]] =
runWithFileArgs(args).provideSomeLayer[R](ZLayer.succeed(FileArgs.default))

def config(newConfig: CliConfig): CliApp[R, E, A]

Expand Down Expand Up @@ -66,7 +69,7 @@ object CliApp {
private def printDocs(helpDoc: HelpDoc): UIO[Unit] =
printLine(helpDoc.toPlaintext(80)).!

def run(args: List[String]): ZIO[R, CliError[E], Option[A]] = {
override def runWithFileArgs(args: List[String]): ZIO[R & FileArgs, CliError[E], Option[A]] = {
def executeBuiltIn(builtInOption: BuiltInOption): ZIO[R, CliError[E], Option[A]] =
builtInOption match {
case ShowHelp(synopsis, helpDoc) =>
Expand Down Expand Up @@ -125,8 +128,11 @@ object CliApp {
case Command.Subcommands(parent, _) => prefix(parent)
}

self.command
.parse(prefix(self.command) ++ args, self.config)
(self.command.names.headOption match {
case Some(name) => ZIO.serviceWithZIO[FileArgs](_.getArgsFromFile(name))
case None => ZIO.succeed(Nil)
})
.flatMap(self.command.parse(prefix(self.command) ++ args, _, self.config))
.foldZIO(
e => printDocs(e.error) *> ZIO.fail(CliError.Parsing(e)),
{
Expand Down
28 changes: 19 additions & 9 deletions zio-cli/shared/src/main/scala/zio/cli/Command.scala
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ sealed trait Command[+A] extends Parameter with Named { self =>

final def orElseEither[B](that: Command[B]): Command[Either[A, B]] = map(Left(_)) | that.map(Right(_))

def parse(args: List[String], conf: CliConfig): IO[ValidationError, CommandDirective[A]]
def parse(
args: List[String],
fromFiles: List[FileArgs.ArgsFromFile],
conf: CliConfig
): IO[ValidationError, CommandDirective[A]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be better to have fromFiles as the third parameter with a default value. Probably it's sign of a bad use, but there might be someone calling directly this method and would be a breaking change. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems reasonable


final def subcommands[B](that: Command[B])(implicit ev: Reducable[A, B]): Command[ev.Out] =
Command.Subcommands(self, that).map(ev.fromTuple2(_))
Expand Down Expand Up @@ -110,14 +114,15 @@ object Command {

def parse(
args: List[String],
fromFiles: List[FileArgs.ArgsFromFile],
conf: CliConfig
): IO[ValidationError, CommandDirective[(OptionsType, ArgsType)]] = {
def parseBuiltInArgs(args: List[String]): IO[ValidationError, CommandDirective[Nothing]] =
if (args.headOption.exists(conf.normalizeCase(_) == conf.normalizeCase(self.name))) {
val options = BuiltInOption
.builtInOptions(self, self.synopsis, self.helpDoc)
Options
.validate(options, args.tail, conf)
.validate(options, args.tail, Nil, conf)
.map(_._3)
.someOrFail(
ValidationError(
Expand Down Expand Up @@ -158,7 +163,7 @@ object Command {
}
tuple1 = splitForcedArgs(commandOptionsAndArgs)
(optionsAndArgs, forcedCommandArgs) = tuple1
tuple2 <- Options.validate(options, optionsAndArgs, conf)
tuple2 <- Options.validate(options, optionsAndArgs, fromFiles, conf)
(optionsError, commandArgs, optionsType) = tuple2
tuple <- self.args.validate(commandArgs ++ forcedCommandArgs, conf).mapError(optionsError.getOrElse(_))
(argsLeftover, argsType) = tuple
Expand Down Expand Up @@ -202,9 +207,10 @@ object Command {

def parse(
args: List[String],
fromFiles: List[FileArgs.ArgsFromFile],
conf: CliConfig
): IO[ValidationError, CommandDirective[B]] =
command.parse(args, conf).map(_.map(f))
command.parse(args, fromFiles, conf).map(_.map(f))

lazy val synopsis: UsageSynopsis = command.synopsis

Expand All @@ -222,9 +228,12 @@ object Command {

def parse(
args: List[String],
fromFiles: List[FileArgs.ArgsFromFile],
conf: CliConfig
): IO[ValidationError, CommandDirective[A]] =
left.parse(args, conf).catchSome { case ValidationError(CommandMismatch, _) => right.parse(args, conf) }
left.parse(args, fromFiles, conf).catchSome { case ValidationError(CommandMismatch, _) =>
right.parse(args, fromFiles, conf)
}

lazy val synopsis: UsageSynopsis = UsageSynopsis.Mixed

Expand Down Expand Up @@ -286,6 +295,7 @@ object Command {

def parse(
args: List[String],
fromFiles: List[FileArgs.ArgsFromFile],
conf: CliConfig
): IO[ValidationError, CommandDirective[(A, B)]] = {
val helpDirectiveForChild = {
Expand All @@ -294,7 +304,7 @@ object Command {
case _ :: tail => tail
}
child
.parse(safeTail, conf)
.parse(safeTail, fromFiles, conf)
.collect(ValidationError(ValidationErrorType.InvalidArgument, HelpDoc.empty)) {
case CommandDirective.BuiltIn(BuiltInOption.ShowHelp(synopsis, helpDoc)) =>
val parentName = names.headOption.getOrElse("")
Expand All @@ -316,7 +326,7 @@ object Command {
case _ :: tail => tail
}
child
.parse(safeTail, conf)
.parse(safeTail, fromFiles, conf)
.collect(ValidationError(ValidationErrorType.InvalidArgument, HelpDoc.empty)) {
case directive @ CommandDirective.BuiltIn(BuiltInOption.ShowWizard(_)) => directive
}
Expand All @@ -326,7 +336,7 @@ object Command {
ZIO.succeed(CommandDirective.builtIn(BuiltInOption.ShowWizard(self)))

parent
.parse(args, conf)
.parse(args, fromFiles, conf)
.flatMap {
case CommandDirective.BuiltIn(BuiltInOption.ShowHelp(_, _)) =>
helpDirectiveForChild orElse helpDirectiveForParent
Expand All @@ -335,7 +345,7 @@ object Command {
case builtIn @ CommandDirective.BuiltIn(_) => ZIO.succeed(builtIn)
case CommandDirective.UserDefined(leftover, a) if leftover.nonEmpty =>
child
.parse(leftover, conf)
.parse(leftover, fromFiles, conf)
.mapBoth(
{
case ValidationError(CommandMismatch, _) =>
Expand Down
61 changes: 61 additions & 0 deletions zio-cli/shared/src/main/scala/zio/cli/FileArgs.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package zio.cli

import zio._
import java.nio.file.Path

trait FileArgs {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change Args for something referring to Options because this represents Options from files and there are both Options and Args in zio-cli.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call

def getArgsFromFile(command: String): UIO[List[FileArgs.ArgsFromFile]]
}
object FileArgs extends FileArgsPlatformSpecific {

final case class ArgsFromFile(path: String, args: List[String])

case object Noop extends FileArgs {
override def getArgsFromFile(command: String): UIO[List[ArgsFromFile]] = ZIO.succeed(Nil)
}

case object Live extends FileArgs {

private def optReadPath(path: Path): UIO[Option[FileArgs.ArgsFromFile]] =
(for {
exists <- ZIO.attempt(path.toFile.exists())
pathString = path.toString
optContents <-
ZIO
.readFile(pathString)
.map(c => FileArgs.ArgsFromFile(pathString, c.split('\n').map(_.trim).filter(_.nonEmpty).toList))
.when(exists)
} yield optContents)
.catchAllCause(ZIO.logErrorCause(s"Error reading options from file '$path', skipping...", _).as(None))

private def getPathAndParents(path: Path): Task[List[Path]] =
for {
parentPath <- ZIO.attempt(Option(path.getParent))
parents <- parentPath match {
case Some(parentPath) => getPathAndParents(parentPath)
case None => ZIO.succeed(Nil)
}
} yield path :: parents

override def getArgsFromFile(command: String): UIO[List[ArgsFromFile]] =
(for {
cwd <- System.property("user.dir")
home <- System.property("user.home")
commandFile = s".$command"

pathsFromCWD <- cwd match {
case Some(cwd) => ZIO.attempt(Path.of(cwd)).flatMap(getPathAndParents)
case None => ZIO.succeed(Nil)
}
homePath <- ZIO.foreach(home)(p => ZIO.attempt(Path.of(p)))
allPaths = (pathsFromCWD ::: homePath.toList).distinct

argsFromFiles <- ZIO.foreach(allPaths) { path =>
ZIO.attempt(path.resolve(commandFile)).flatMap(optReadPath)
}
} yield argsFromFiles.flatten)
.catchAllCause(ZIO.logErrorCause(s"Error reading options from files, skipping...", _).as(Nil))

}

}
Loading
Loading