-
Notifications
You must be signed in to change notification settings - Fork 79
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
[Reopen] Read args from file, merge args and pass to the cmd. #308
base: master
Are you sure you want to change the base?
Changes from all commits
ca2471a
bb2461b
ea35be5
f8cba2a
9fbc76d
79ea5a5
3fd0278
dea5f46
07d1823
952fde7
83a8f6f
d655dee
7a6d2cc
40d0dcf
88489a6
ba5aa9d
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 |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package zio.cli | ||
|
||
import java.io.IOException | ||
import zio._ | ||
|
||
object ConfigFileArgsPlatformSpecific extends ConfigFilePlatformSpecific { | ||
def findPathsOfCliConfigFiles(topLevelCommand: String): Task[List[String]] = | ||
ZIO.succeed(Nil) // Always return empty for JS | ||
|
||
def loadOptionsFromConfigFiles(topLevelCommand: String): ZIO[Any, IOException, List[String]] = | ||
ZIO.succeed(Nil) // Always return empty for JS | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
package zio.cli | ||
|
||
import java.io.IOException | ||
import zio._ | ||
import java.nio.file.{Files, Path, Paths} | ||
import scala.io.Source | ||
|
||
object ConfigFileArgsPlatformSpecific extends ConfigFilePlatformSpecific { | ||
def findPathsOfCliConfigFiles(topLevelCommand: String): Task[List[String]] = { | ||
val filename = s".$topLevelCommand" | ||
val cwd = java.lang.System.getProperty("user.dir") | ||
val homeDirOpt = java.lang.System.getProperty("user.home") | ||
Comment on lines
+11
to
+12
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. See: |
||
|
||
def parentPaths(path: String): List[String] = { | ||
val parts = path.split(java.io.File.separatorChar).filterNot(_.isEmpty) | ||
(0 to parts.length) | ||
.map(i => s"${java.io.File.separatorChar}${parts.take(i).mkString(java.io.File.separator)}") | ||
.toList | ||
} | ||
Comment on lines
+14
to
+19
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. Does this work for |
||
|
||
val paths = parentPaths(cwd) | ||
val pathsToCheck = homeDirOpt :: homeDirOpt :: paths | ||
|
||
// Use ZIO to filter the paths | ||
for { | ||
doPathExist <- ZIO.foreach(pathsToCheck)(path => ZIO.succeed(Files.exists(Path.of(path, filename)))) | ||
existingPaths = doPathExist.zip(pathsToCheck).collect { case (exists, path) if exists => path } | ||
} yield existingPaths.distinct // Use distinct to remove duplicates at the end | ||
} | ||
|
||
def loadOptionsFromConfigFiles(topLevelCommand: String): ZIO[Any, IOException, List[String]] = | ||
for { | ||
filePaths <- findPathsOfCliConfigFiles(topLevelCommand) | ||
.refineToOrDie[IOException] | ||
lines <- ZIO | ||
.foreach(filePaths) { filePath => | ||
ZIO.acquireReleaseWith( | ||
ZIO.attempt( | ||
Source.fromFile(Paths.get(filePath, "." + topLevelCommand).toFile) | ||
) | ||
)(source => ZIO.attempt(source.close()).orDie) { source => | ||
ZIO.attempt(source.getLines().toList.filter(_.trim.nonEmpty)) | ||
} | ||
} | ||
.map(_.flatten) | ||
Comment on lines
+35
to
+45
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.
|
||
.refineToOrDie[IOException] | ||
} yield lines | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,197 @@ | ||
import zio._ | ||
import zio.test._ | ||
import zio.test.Assertion._ | ||
import java.nio.file.{Files, Paths, Path} | ||
import java.io.IOException | ||
import zio.cli.ConfigFileArgsPlatformSpecific | ||
import zio.cli.ConfigFilePlatformSpecific | ||
import zio.cli.Options | ||
import zio.cli.Args | ||
import zio.cli.Exists | ||
import zio.cli.Command | ||
import zio.Console.printLine | ||
import zio.stream.ZSink | ||
import zio.stream.ZPipeline | ||
import zio.stream.ZStream | ||
import zio.cli.CliApp | ||
import zio.cli.HelpDoc.Span.text | ||
|
||
object FileBasedArgs extends ZIOSpecDefault { | ||
|
||
val configFileOps: ConfigFilePlatformSpecific = ConfigFileArgsPlatformSpecific | ||
|
||
def spec = suite("FileBasedArgs")( | ||
test("Full CLI App output test") { | ||
for { | ||
// Create Sample config files | ||
cwd <- ZIO.succeed(Paths.get(java.lang.System.getProperty("user.dir"))) | ||
homeDir <- ZIO.succeed(Paths.get(java.lang.System.getProperty("user.home"))) | ||
command <- ZIO.succeed("wc") | ||
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. command = "wc" |
||
|
||
_ <- createLineCountTestFile(cwd, "sample_file") | ||
_ <- createSampleConfigFiles(cwd, homeDir, command, content_home = "-w", content_cwd = "-l") | ||
|
||
_ <- cliApp.run(args = List("sample_file")).either | ||
output <- TestConsole.output | ||
|
||
correctOutput <- ZIO.succeed(" 3 4 20 sample_file\n") | ||
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.
|
||
|
||
_ <- cleanupLineCountTestFile(cwd, "sample_file") | ||
_ <- cleanUpSampleConfigFiles(cwd: Path, homeDir: Path, command) | ||
} yield assert(output.last)(equalTo(correctOutput)) | ||
}, | ||
test("should load options from files and merge them appropriatly") { | ||
for { | ||
// Create Sample config files | ||
cwd <- ZIO.succeed(Paths.get(java.lang.System.getProperty("user.dir"))) | ||
homeDir <- ZIO.succeed(Paths.get(java.lang.System.getProperty("user.home"))) | ||
command <- ZIO.succeed("testApp") | ||
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. command = "testApp" |
||
_ <- createSampleConfigFiles(cwd, homeDir, command) | ||
|
||
// Check if the func checkAndGetOptionsFilePaths can | ||
configArgs <- configFileOps.loadOptionsFromConfigFiles(command) | ||
|
||
_ <- cleanUpSampleConfigFiles(cwd: Path, homeDir: Path, command) | ||
|
||
} yield assert(configArgs)(hasSameElements(List("home=true", "dir=true", "home=false"))) | ||
}, | ||
test("should return directory ~/home and ./ which have .testApp config file for loading the args") { | ||
for { | ||
// Create Sample config files | ||
cwd <- ZIO.succeed(Paths.get(java.lang.System.getProperty("user.dir"))) | ||
homeDir <- ZIO.succeed(Paths.get(java.lang.System.getProperty("user.home"))) | ||
command <- ZIO.succeed("testApp1") | ||
_ <- createSampleConfigFiles(cwd, homeDir, command) | ||
|
||
// Check if the func checkAndGetOptionsFilePaths can | ||
paths <- configFileOps.findPathsOfCliConfigFiles(command) | ||
|
||
_ <- cleanUpSampleConfigFiles(cwd: Path, homeDir: Path, command) | ||
|
||
} yield assert(paths)(hasSameElements(List(homeDir.toString(), cwd.toString()))) | ||
} | ||
) | ||
|
||
def createLineCountTestFile( | ||
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. This should return ZIO[Scope, IOException, Unit] and clean itself up |
||
cwd: Path, | ||
file_name: String = "sample_file", | ||
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.
|
||
content: String = "asdf\nqweqwer\njdsafn" | ||
): IO[IOException, Unit] = | ||
ZIO.attempt { | ||
Files.write(Paths.get(cwd.toString(), s"$file_name"), java.util.Arrays.asList(content)); | ||
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.
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.
|
||
() | ||
}.refineToOrDie[IOException] | ||
|
||
def cleanupLineCountTestFile(cwd: Path, file_name: String = "sample_file"): IO[IOException, Unit] = | ||
ZIO.attempt { | ||
Files.delete(Paths.get(cwd.toString(), s"$file_name")); | ||
() | ||
}.refineToOrDie[IOException] | ||
|
||
def createSampleConfigFiles( | ||
cwd: Path, | ||
homeDir: Path, | ||
command: String = "testApp", | ||
content_home: String = "home=true", | ||
content_cwd: String = "dir=true\nhome=false" | ||
Comment on lines
+95
to
+96
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. camel 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. Also, is it possible to remove the default arguments? Also, I imagine that there should be multiple tests asserting expected behavior for different permutations of hierarchy overrides. |
||
): IO[IOException, Unit] = | ||
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. same thing as above, writing to file should return zio |
||
ZIO.attempt { | ||
Files.write(Paths.get(homeDir.toString(), s".$command"), java.util.Arrays.asList(content_home)); | ||
Files.write(Paths.get(cwd.toString(), s".$command"), java.util.Arrays.asList(content_cwd)); | ||
() | ||
}.refineToOrDie[IOException] | ||
|
||
def cleanUpSampleConfigFiles(cwd: Path, homeDir: Path, command: String = "testApp"): IO[IOException, Unit] = | ||
ZIO.attempt { | ||
Files.delete(Paths.get(homeDir.toString(), s".$command")); | ||
Files.delete(Paths.get(cwd.toString(), s".$command")); | ||
() | ||
}.refineToOrDie[IOException] | ||
|
||
val bytesFlag: Options[Boolean] = Options.boolean("c") | ||
val linesFlag: Options[Boolean] = Options.boolean("l") | ||
val wordsFlag: Options[Boolean] = Options.boolean("w") | ||
val charFlag: Options[Boolean] = Options.boolean("m", false) | ||
|
||
case class WcOptions(bytes: Boolean, lines: Boolean, words: Boolean, char: Boolean) | ||
case class WcResult( | ||
fileName: String, | ||
countBytes: Option[Long], | ||
countLines: Option[Long], | ||
countWords: Option[Long], | ||
countChar: Option[Long] | ||
) | ||
|
||
val options = (bytesFlag ++ linesFlag ++ wordsFlag ++ charFlag).as(WcOptions.apply _) | ||
|
||
val args = Args.file("files", Exists.Yes).repeat1 | ||
|
||
val wc: Command[(WcOptions, ::[Path])] = Command("wc", options, args) | ||
|
||
val execute: (WcOptions, ::[Path]) => UIO[Unit] = { | ||
def printResult(res: List[WcResult]): UIO[Unit] = { | ||
def wcTotal(results: List[WcResult]) = { | ||
def optSum(acc: WcResult, elem: WcResult, extract: WcResult => Option[Long]): Option[Long] = | ||
extract(acc).flatMap(a => extract(elem).map(_ + a)) | ||
|
||
results.fold(WcResult("total", Some(0), Some(0), Some(0), Some(0))) { (acc, wcRes) => | ||
acc.copy( | ||
countBytes = optSum(acc, wcRes, _.countBytes), | ||
countChar = optSum(acc, wcRes, _.countChar), | ||
countWords = optSum(acc, wcRes, _.countWords), | ||
countLines = optSum(acc, wcRes, _.countLines) | ||
) | ||
} | ||
} | ||
|
||
def format(res: WcResult) = { | ||
def opt(option: Option[Long]): String = option.map(l => f"$l%9d").getOrElse("") | ||
|
||
s"${opt(res.countLines)} ${opt(res.countWords)} ${opt(res.countChar)} ${opt(res.countBytes)} ${res.fileName}" | ||
} | ||
|
||
ZIO.foreachDiscard(res)(r => printLine(format(r)).!) *> ZIO | ||
.when(res.length > 1)(printLine(format(wcTotal(res))).!) | ||
.ignore | ||
} | ||
|
||
(opts, paths) => { | ||
zio.Console.printLine(s"executing wc with args: $opts $paths").! *> | ||
ZIO | ||
.foreachPar[Any, Throwable, Path, WcResult, List](paths)({ path => | ||
def option(bool: Boolean, sink: ZSink[Any, Throwable, Byte, Byte, Long]) | ||
: ZSink[Any, Throwable, Byte, Byte, Option[Long]] = | ||
if (bool) sink.map(Some(_)) else ZSink.succeed(None) | ||
|
||
val byteCount = option(opts.bytes, ZSink.count) | ||
val lineCount = option(opts.lines, ZPipeline.utfDecode >>> ZPipeline.splitLines >>> ZSink.count) | ||
val wordCount = | ||
option( | ||
opts.words, | ||
ZPipeline.utfDecode >>> ZPipeline.mapChunks((_: Chunk[String]).flatMap(_.split("\\s+"))) >>> ZSink.count | ||
) | ||
val charCount = | ||
option(opts.char, ZPipeline.utfDecode >>> ZSink.foldLeft[String, Long](0L)((s, e) => s + e.length)) | ||
|
||
val zippedSinks | ||
: ZSink[Any, Throwable, Byte, Byte, (Option[Long], Option[Long], Option[Long], Option[Long])] = | ||
(byteCount <&> lineCount <&> wordCount <&> charCount) | ||
|
||
ZStream | ||
.fromFile(path.toFile) | ||
.run(zippedSinks) | ||
.map(t => WcResult(path.getFileName.toString, t._1, t._2, t._3, t._4)) | ||
}) | ||
.withParallelism(4) | ||
.orDie | ||
.flatMap(res => printResult(res)) | ||
} | ||
} | ||
|
||
val cliApp = CliApp.make[Any, Throwable, (WcOptions, ::[Path]), Unit]( | ||
"ZIO Word Count", | ||
"0.1.2", | ||
text("counts words in the file"), | ||
wc | ||
)(execute.tupled) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
package zio.cli | ||
|
||
import java.io.IOException | ||
import zio._ | ||
import java.nio.file.{Files, Path, Paths} | ||
import scala.io.Source | ||
|
||
object ConfigFileArgsPlatformSpecific extends ConfigFilePlatformSpecific { | ||
def findPathsOfCliConfigFiles(topLevelCommand: String): Task[List[String]] = { | ||
val filename = s".$topLevelCommand" | ||
val cwd = java.lang.System.getProperty("user.dir") | ||
val homeDirOpt = java.lang.System.getProperty("user.home") | ||
|
||
def parentPaths(path: String): List[String] = { | ||
val parts = path.split(java.io.File.separatorChar).filterNot(_.isEmpty) | ||
(0 to parts.length) | ||
.map(i => s"${java.io.File.separatorChar}${parts.take(i).mkString(java.io.File.separator)}") | ||
.toList | ||
} | ||
|
||
val paths = parentPaths(cwd) | ||
val pathsToCheck = homeDirOpt :: homeDirOpt :: paths | ||
|
||
// Use ZIO to filter the paths | ||
for { | ||
doPathExist <- ZIO.foreach(pathsToCheck)(path => ZIO.succeed(Files.exists(Path.of(path, filename)))) | ||
existingPaths = doPathExist.zip(pathsToCheck).collect { case (exists, path) if exists => path } | ||
} yield existingPaths.distinct // Use distinct to remove duplicates at the end | ||
} | ||
|
||
def loadOptionsFromConfigFiles(topLevelCommand: String): ZIO[Any, IOException, List[String]] = | ||
for { | ||
filePaths <- findPathsOfCliConfigFiles(topLevelCommand) | ||
.refineToOrDie[IOException] | ||
lines <- ZIO | ||
.foreach(filePaths) { filePath => | ||
ZIO.acquireReleaseWith( | ||
ZIO.attempt( | ||
Source.fromFile(Paths.get(filePath, "." + topLevelCommand).toFile) | ||
) | ||
)(source => ZIO.attempt(source.close()).orDie) { source => | ||
ZIO.attempt(source.getLines().toList.filter(_.trim.nonEmpty)) | ||
} | ||
} | ||
.map(_.flatten) | ||
.refineToOrDie[IOException] | ||
} yield lines | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -128,19 +128,30 @@ object CliApp { | |
case Command.Subcommands(parent, _) => prefix(parent) | ||
} | ||
|
||
self.command | ||
.parse(prefix(self.command) ++ args, self.config) | ||
.foldZIO( | ||
e => printDocs(e.error) *> ZIO.fail(CliError.Parsing(e)), | ||
{ | ||
case CommandDirective.UserDefined(_, value) => | ||
self.execute(value).map(Some(_)).mapError(CliError.Execution(_)) | ||
case CommandDirective.BuiltIn(x) => | ||
executeBuiltIn(x).catchSome { case err @ CliError.Parsing(e) => | ||
printDocs(e.error) *> ZIO.fail(err) | ||
} | ||
// Reading args from config files and combining with provided args | ||
val combinedArgs: ZIO[R, CliError[E], List[String]] = | ||
ConfigFileArgsPlatformSpecific | ||
.loadOptionsFromConfigFiles(self.command.names.head) | ||
.flatMap { configArgs => | ||
ZIO.succeed(configArgs ++ args) | ||
} | ||
Comment on lines
+135
to
137
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.
|
||
) | ||
.mapError(e => CliError.IO(e)) | ||
|
||
combinedArgs.flatMap { allArgs => | ||
self.command | ||
.parse(prefix(self.command) ++ allArgs, self.config) | ||
.foldZIO( | ||
e => printDocs(e.error) *> ZIO.fail(CliError.Parsing(e)), | ||
{ | ||
case CommandDirective.UserDefined(_, value) => | ||
self.execute(value).map(Some(_)).mapError(CliError.Execution(_)) | ||
case CommandDirective.BuiltIn(x) => | ||
executeBuiltIn(x).catchSome { case err @ CliError.Parsing(e) => | ||
printDocs(e.error) *> ZIO.fail(err) | ||
} | ||
} | ||
) | ||
} | ||
} | ||
|
||
override def flatMap[R1 <: R, E1 >: E, B](f: A => ZIO[R1, E1, B]): CliApp[R1, E1, B] = | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package zio.cli | ||
|
||
import zio._ | ||
import java.io.IOException | ||
|
||
trait ConfigFilePlatformSpecific { | ||
def findPathsOfCliConfigFiles(topLevelCommand: String): Task[List[String]] | ||
def loadOptionsFromConfigFiles(topLevelCommand: String): ZIO[Any, IOException, List[String]] | ||
} | ||
Comment on lines
+6
to
+9
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. Personal opinion: trait ConfigFileArgs {
// ...
}
object ConfigFileArgs {
val default: ConfigFileArgs // platform specific
} sealed trait CliApp[-R, +E, +A] { self =>
def runWithFileArgs(args: List[String]): ZIO[R & ConfigFileArgs, CliError[E], Option[A]]
final def run(args: List[String]): ZIO[R, CliError[E], Option[A]] =
runWithFileArgs(args).provideSomeLayer(ZLayer.succeed(ConfigFileArgs.default))
} 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. The reason I suggest this is because it would allow for more separation of concerns, and make much more testable. Again, very much personal opinion 😅 |
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 to be copy-pasted, if so, move into
./zio-cli/jvm-native