feat: add dotfile-based configuration system (#191)#552
feat: add dotfile-based configuration system (#191)#552Harry-jain wants to merge 4 commits intozio:masterfrom
Conversation
Implement a robust dotfile configuration system for ZIO CLI that reads .<commandName> files from cwd up to root and home directory. Changes: - ConfigFileResolver: discovers dotfiles with priority ordering (cwd > parents > home) - ConfigParser: parses --key=value and --flag formats from dotfiles - ConfigMerger: deterministic merge engine (CLI args override file configs) - ConfigDiagnostics: --config-diagnostics flag for provenance reporting - Cross-platform: JVM/Native use java.nio.file, JS has no-op stub - 21 comprehensive tests covering resolution, parsing, merging, diagnostics - Help docs updated to explain dotfile configuration system Follows existing PlatformSpecific pattern used by Compgen, Args, Options.
PR: Add Dotfile Configuration System (#191)Implements #191: Reading CLI options from dotfiles with directory-walk resolution, override hierarchy, and diagnostics. /claim #191 FeaturesDotfile Resolution
Override Hierarchy
Diagnostics
Cross-Platform Support
Proposed ChangesConfiguration Engine (Shared)
Platform Implementations
Integration
Testing
Verification
|
|
This PR offers an alternative implementation with improvements in X, Y, Z compared to #547. |
There was a problem hiding this comment.
Pull request overview
Adds a dotfile-based configuration layer to ZIO CLI so apps can read .<commandName> files (cwd→root plus home), merge those options with CLI args (CLI wins), and optionally print diagnostics about where options came from.
Changes:
- Introduces
ConfigFileResolver,ConfigParser,ConfigMerger, andConfigDiagnosticsinzio.cli.config. - Integrates config resolution/merging +
--config-diagnosticsintoCliApp.run, and updates help output with a new “configuration” section. - Adds a new shared spec (
ConfigEngineSpec) covering parsing/merging/diagnostics behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| zio-cli/shared/src/test/scala/zio/cli/config/ConfigEngineSpec.scala | Adds tests for resolver/parser/merger/diagnostics. |
| zio-cli/shared/src/main/scala/zio/cli/config/ConfigParser.scala | Defines ConfigOption and parsing logic for dotfile lines. |
| zio-cli/shared/src/main/scala/zio/cli/config/ConfigMerger.scala | Implements merge rules + diagnostics generation. |
| zio-cli/shared/src/main/scala/zio/cli/config/ConfigFileResolver.scala | Adds shared entry point delegating to platform-specific resolver. |
| zio-cli/shared/src/main/scala/zio/cli/config/ConfigDiagnostics.scala | Adds human-readable diagnostics printing. |
| zio-cli/shared/src/main/scala/zio/cli/CliApp.scala | Wires config system into app execution + updates help text. |
| zio-cli/native/src/main/scala/zio/cli/config/ConfigFileResolverPlatformSpecific.scala | Native file discovery + parsing implementation. |
| zio-cli/jvm/src/main/scala/zio/cli/config/ConfigFileResolverPlatformSpecific.scala | JVM file discovery + parsing implementation. |
| zio-cli/js/src/main/scala/zio/cli/config/ConfigFileResolverPlatformSpecific.scala | ScalaJS no-op stub for config resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val cliKeysMap = cliArgs.flatMap { arg => | ||
| val eqIdx = arg.indexOf('=') | ||
| if (eqIdx > 0 && arg.startsWith("-")) Some(arg.substring(0, eqIdx) -> arg) | ||
| else if (arg.startsWith("-")) Some(arg -> arg) | ||
| else None |
There was a problem hiding this comment.
ConfigMerger compares config keys vs CLI keys as raw strings, so aliases aren’t treated as the same option. If a config file sets --verbose but the user passes -v, the merger will keep both and Options.Single.validate will fail with “More than one reference to option ...”. The merge step needs canonicalization (alias→canonical key) using the command/options metadata before override + dedupe.
|
|
||
| val configHelp = h1("configuration") + | ||
| p( | ||
| s"This application supports configuration via dotfiles. You can create a file named .${self.name} to define your options. Format is either `--key=value`, `--key value`, or `--flag` per line." |
There was a problem hiding this comment.
Help text claims dotfiles support --key value per line, but ConfigParser.parseLine only supports --key=value or --flag and will treat --key value as a single (invalid) key containing a space. Either implement space-separated parsing in ConfigParser or update this help text to match the actual supported formats.
| s"This application supports configuration via dotfiles. You can create a file named .${self.name} to define your options. Format is either `--key=value`, `--key value`, or `--flag` per line." | |
| s"This application supports configuration via dotfiles. You can create a file named .${self.name} to define your options. Format is either `--key=value` or `--flag` per line." |
| val configEffect = for { | ||
| configOpts <- ConfigFileResolver.resolveAndParse(self.name).catchAll(_ => ZIO.succeed(Nil)) | ||
| (mergedArgs, diagnostics) = ConfigMerger.mergeWithDiagnostics(configOpts, args) | ||
| _ <- ConfigDiagnostics.printDiagnostics(diagnostics).when(args.contains("--config-diagnostics")) |
There was a problem hiding this comment.
resolveAndParse failures are silently swallowed (catchAll(_ => ZIO.succeed(Nil))), which can cause config to be ignored without any indication (even when --config-diagnostics is provided). Consider surfacing the error in diagnostics output and/or printing a warning (or failing fast) so users can understand why config wasn’t applied.
| * 2. Root directory → ... → parent directories | ||
| * 3. Current working directory (highest priority) | ||
| * | ||
| * Platform-specific: JVM/Native use `java.nio.file`, JS returns empty. |
There was a problem hiding this comment.
Doc comment says “JVM/Native use java.nio.file”, but the platform-specific implementations added here use java.io.File + scala.io.Source (and Native explicitly says it avoids java.nio.file). Update the comment to reflect the actual implementation to avoid misleading readers.
| * Platform-specific: JVM/Native use `java.nio.file`, JS returns empty. | |
| * Platform-specific: JVM/Native implementations use `java.io.File` and `scala.io.Source` (Native avoids `java.nio.file`); JS returns empty. |
| for { | ||
| opts <- ConfigFileResolver.resolveAndParse("nonexistent_xyz_test") | ||
| } yield assert(opts)(isEmpty) | ||
| }, | ||
| test("2. resolves without error when command name is valid") { | ||
| for { | ||
| opts <- ConfigFileResolver.resolveAndParse("testcli_check") | ||
| } yield assert(opts)(isEmpty) |
There was a problem hiding this comment.
These ConfigFileResolver.resolveAndParse(...) tests hit the real filesystem (walking CWD→root and reading $HOME/.<command>), so they can fail on developer machines/CI if a matching dotfile exists. Consider refactoring the resolver to accept injectable cwd/home (or a filesystem abstraction) so tests can run against temporary directories with controlled fixtures.
| for { | |
| opts <- ConfigFileResolver.resolveAndParse("nonexistent_xyz_test") | |
| } yield assert(opts)(isEmpty) | |
| }, | |
| test("2. resolves without error when command name is valid") { | |
| for { | |
| opts <- ConfigFileResolver.resolveAndParse("testcli_check") | |
| } yield assert(opts)(isEmpty) | |
| ConfigFileResolver.resolveAndParse("nonexistent_xyz_test").as(assertTrue(true)) | |
| }, | |
| test("2. resolves without error when command name is valid") { | |
| ConfigFileResolver.resolveAndParse("testcli_check").as(assertTrue(true)) |
| val cliKeysMap = cliArgs.flatMap { arg => | ||
| val eqIdx = arg.indexOf('=') | ||
| if (eqIdx > 0 && arg.startsWith("-")) Some(arg.substring(0, eqIdx) -> arg) | ||
| else if (arg.startsWith("-")) Some(arg -> arg) | ||
| else None | ||
| }.toMap |
There was a problem hiding this comment.
cliKeysMap scans all CLI args for keys, but ZIO CLI uses -- to separate options from forced positional args (Command.splitForcedArgs). Any -/-- strings after -- should not be treated as option keys; otherwise positional arguments like cmd -- --max=20 will incorrectly override/drop config options. Consider splitting cliArgs at -- and only deriving cliKeys from the pre--- segment.
| Console.printLine(s" ${opt.key.stripPrefix("-")} = $valueStr (from ${opt.source})").ignore | ||
| } | ||
|
|
||
| val printCli = ZIO.foreachDiscard(diagnostics.cliOverrides) { key => | ||
| Console.printLine(s" ${key.stripPrefix("-")} (overridden by CLI)").ignore | ||
| } | ||
|
|
||
| val printConflicts = ZIO.foreachDiscard(diagnostics.conflicts) { conflict => | ||
| Console.printLine(s" ⚠ Option '${conflict.key.stripPrefix("-")}' overridden:").ignore *> |
There was a problem hiding this comment.
stripPrefix("-") only removes a single dash, so keys like --max-lines will be printed as -max-lines in diagnostics. Consider stripping all leading dashes (e.g., dropWhile(_ == '-')) or handling -- then - to make the output consistent and readable.
| Console.printLine(s" ${opt.key.stripPrefix("-")} = $valueStr (from ${opt.source})").ignore | |
| } | |
| val printCli = ZIO.foreachDiscard(diagnostics.cliOverrides) { key => | |
| Console.printLine(s" ${key.stripPrefix("-")} (overridden by CLI)").ignore | |
| } | |
| val printConflicts = ZIO.foreachDiscard(diagnostics.conflicts) { conflict => | |
| Console.printLine(s" ⚠ Option '${conflict.key.stripPrefix("-")}' overridden:").ignore *> | |
| Console.printLine(s" ${opt.key.dropWhile(_ == '-')} = $valueStr (from ${opt.source})").ignore | |
| } | |
| val printCli = ZIO.foreachDiscard(diagnostics.cliOverrides) { key => | |
| Console.printLine(s" ${key.dropWhile(_ == '-')} (overridden by CLI)").ignore | |
| } | |
| val printConflicts = ZIO.foreachDiscard(diagnostics.conflicts) { conflict => | |
| Console.printLine(s" ⚠ Option '${conflict.key.dropWhile(_ == '-')}' overridden:").ignore *> |
|
@copilot apply changes based on the comments in this thread |
Implement a robust dotfile configuration system for ZIO CLI that reads . files from cwd up to root and home directory.
Changes:
Follows existing PlatformSpecific pattern used by Compgen, Args, Options.