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

Read args from file, merge args and pass to the cmd. #278

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
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
73 changes: 72 additions & 1 deletion zio-cli/shared/src/main/scala/zio/cli/CliApp.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import zio.cli.HelpDoc.Span.{code, text}
import zio.cli.HelpDoc.{h1, p}
import zio.cli.completion.{Completion, CompletionScript}
import zio.cli.figlet.FigFont
import java.nio.file.{Files, Paths}

import scala.annotation.tailrec

Expand Down Expand Up @@ -56,6 +57,75 @@ object CliApp {
def printDocs(helpDoc: HelpDoc): UIO[Unit] =
printLine(helpDoc.toPlaintext(80)).!

def checkAndGetOptionsFilePaths(topLevelCommand: String): List[String] = {
Copy link
Member

Choose a reason for hiding this comment

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

This is side-effecting, so should return Task[List[String]] for consistency with the rest of ZIO ecosystem.

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 :: paths

val existingFiles = pathsToCheck.filter { path =>
val filePath = Paths.get(path, filename)
Files.exists(filePath)
}

// Print out the paths with existing files
existingFiles.foreach(println)

return existingFiles
}

def loadOptionsFromFile(
topLevelCommand: String
): List[String] = {
// Get a list of file paths based on the top-level command
val filePaths = checkAndGetOptionsFilePaths(topLevelCommand)

// Read the contents of the files at the given file paths
val options: List[String] = filePaths.flatMap { filePath =>
val source = scala.io.Source.fromFile(filePath)
source.getLines().toList
Copy link
Member

Choose a reason for hiding this comment

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

This creates a leak because the source is never closed. Also, it's an inefficient way to read a file. Better to use something like ZIO.readFile.

}

// Merges a list of options, removing any duplicate keys.
// If there are options with the same keys but different values, it will use the value from the last option in the
// list.
def mergeOptionsBasedOnPriority(options: List[String]): List[String] = {
val mergedOptions = options.flatMap { opt =>
opt.split('=') match {
case Array(key) => Some(key -> None)
case Array(key, value) => Some(key -> value)
case _ =>
None // handles the case when there isn't exactly one '=' in the string
}
}.toMap.toList.map {
case (key, None) => key
case (key, value) => s"$key=$value"
}

mergedOptions
}

// Merge the read options
val merged = mergeOptionsBasedOnPriority(options)

// Return the merged options
return merged
}
def run(args: List[String]): ZIO[R, Any, Any] = {
def executeBuiltIn(builtInOption: BuiltInOption): ZIO[R, Any, Any] =
builtInOption match {
Expand Down Expand Up @@ -115,8 +185,9 @@ object CliApp {
case Command.Subcommands(parent, _) => prefix(parent)
}

val arg_from_config_files = loadOptionsFromFile(self.name)
Copy link
Member

Choose a reason for hiding this comment

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

camelCase please.

self.command
.parse(prefix(self.command) ++ args, self.config)
.parse(prefix(self.command) ++ arg_from_config_files ++ args, self.config)
Copy link
Member

Choose a reason for hiding this comment

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

Hi! I don't think including arg_from_config_files there would work. It's probably best to do this inside command.parse so you can have more control in how args are parsed.

Copy link
Author

Choose a reason for hiding this comment

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

ok move it inside command.parse

.foldZIO(
e => printDocs(e.error) *> ZIO.fail(e),
{
Expand Down
Loading