fix(config): port working jtc config set/show from JuliaPkgTemplatesCLI#6
Merged
ultimatile merged 15 commits intomainfrom Apr 26, 2026
Merged
fix(config): port working jtc config set/show from JuliaPkgTemplatesCLI#6ultimatile merged 15 commits intomainfrom
ultimatile merged 15 commits intomainfrom
Conversation
The config set subcommand declared no arguments, so jtc config set --author ...
returned "unrecognized option". ConfigCommand.execute also looked up
%SUBCOMMAND% (ArgParse uses %COMMAND%) and never read the nested subcommand
args, so even reachable paths defaulted to show.
Port the option surface from the Python source (JuliaPkgTemplatesCLI):
- Add --author (repeatable), --user, --mail, --license, --julia-version,
--mise-filename-base, --config-file, and exclusive --with-mise/--no-mise
to config set
- Add --config-file to config show
- Generalize add_dynamic_plugin_options! so config set also gets per-plugin
options (skipping License to avoid colliding with --license, using
nargs='?' so --git "ignore=... ssh=true" works)
- Teach ConfigManager.{load,save,get_config_path} to honor a custom path
- Rework ConfigCommand.execute to read %COMMAND%, descend into the nested
args dict, map CLI options into the [default] section, and resolve
plugin keys to their PkgTemplates canonical names (booleans, numerics,
bracket arrays, and comma-split strings round-trip into TOML)
`config set --author A --author B` and `config set --license MIT` persist
values that the create flow ignored: a Vector author was wrapped in another
list (PkgTemplates rejects Vector{Vector{String}}), and license_type was
never promoted to the License plugin so the package shipped with no license.
Pass through Vector authors as-is, and lift `default.license_type` into
plugin_options["License"]["name"] when no CLI license is supplied. Run the
transforms before the dry-run branch so the plan reflects what create will
actually pass to PkgTemplates.
…-mise override Two follow-ups exposed by the new config set surface: - parse_plugin_option_value coerced "1.2"/"1.10" to Float64, so a persisted ProjectFile.version made create fail with "Cannot convert Float64 to VersionNumber" and silently dropped trailing zeros. Match the Python port (int-only coercion via isdigit) and let decimals stay as strings. - create read the mise toggle from merged_options["with_mise"] but the CLI flag landed under "with-mise"/"no-mise", so `create --with-mise` could not override a config that had `with_mise = false`. Normalize the dashed flags into the underscore key before the dry-run/check, so explicit CLI input always wins over the saved default.
- Add --mail to create and use it during package generation: when authors do not already carry an email, append "<mail>" so the generated Project.toml records "Name <mail>". Previously --mail was advertised by config set but silently dropped on create. - Stop dropping dot-notation keys in _apply_set_args. Direct callers that pass mixed shapes like author="A" together with "formatter.style"="blue" used to lose the dotted entry once the CLI-style branch was taken; handle the dotted case alongside the canonical-plugin branch so legacy flat updates round-trip.
…gin fields
PkgTemplates plugins like TagBot declare `token`, `ssh`, and `gpg` as
`Secret`/`Union{Nothing, Secret}`, and the constructor refuses to convert
plain strings. After config set persists `--tagbot token=...`, create then
forwarded the raw String and failed with MethodError.
Reflect on the plugin's field types and wrap strings in PkgTemplates.Secret
when (and only when) the field requires it. Skip fields that don't exist on
the plugin or that already accept a String, so plain options like
License.name keep flowing through unchanged.
parse_plugin_option_value stripped surrounding quotes and then split on commas, so a literal value such as `--git 'name="Doe, Jane"'` was rewritten to ["Doe", "Jane"] and broke String-typed fields like Git.name on create. Track whether the original value was quoted and only apply comma-to-array promotion when the user did not explicitly opt into a string.
There was a problem hiding this comment.
Pull request overview
Fixes the jtc config command wiring so config set/config show have the intended option surface, correctly dispatch via ArgParse’s %COMMAND%, persist defaults (including plugin defaults) to TOML, and ensure create consumes the new persisted shapes end-to-end.
Changes:
- Added/ported ArgParse options for
config show/config set(including--config-file, mise toggles, and dynamic per-plugin options). - Reworked
ConfigCommand.executeto correctly dispatch, parse nested subcommand args, map basic keys, and normalize plugin defaults into canonical PkgTemplates plugin sections. - Updated config loading/saving to accept an optional custom path, and updated
createto consume vector authors, apply--mail, promotelicense_type, and normalize mise overrides; plugin instantiation now wraps Secret-typed fields.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/cli.jl | Adds CLI surface for config show/config set, mise toggles, and dynamic plugin options for config persistence. |
| src/config_command.jl | Fixes ArgParse dispatch (%COMMAND%), implements config-set mapping/parsing (including plugin KV parsing) and stable TOML formatting. |
| src/config_manager.jl | Adds optional custom config path support to get/load/save for --config-file. |
| src/create_command.jl | Updates create flow to consume new persisted shapes (authors vector, mail appending, license promotion, mise override normalization). |
| src/package_generator.jl | Wraps stored strings into PkgTemplates.Secret when plugin field types require it. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot review on the config-set port flagged that "1"/"0" lived in the boolean synonym lists, so a value like `--git indent=1` was persisted as `indent = true` instead of the integer 1. Drop them so numeric plugin options round-trip correctly; "true"/"yes"/"false"/"no" still cover boolean intent. Also bring the get_config_path/load_config/save_config docstrings in line with the new optional `custom_path` parameter so the `--config-file` behavior is discoverable from the API docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two follow-ups from Copilot's second review: - add_dynamic_plugin_options! flattened the create vs config-set paths so both used `nargs='?'` for non-argumentless plugins. CreateCommand.parse_plugin_options only reads plugin keys when the parsed value is a Vector, so any future non-argumentless plugin would be silently dropped on create. Split the branches: create keeps the legacy `:append_arg`/'*' shape, config set keeps the optional KEY=VALUE form. - ConfigCommand.execute called _plugin_canonical_names() inside the uses_cli_shape `any(...)` generator, re-running plugin discovery for every key in sub_args. Lift it into a local so it runs once.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… branches Two cleanup follow-ups from Copilot review: - _BASIC_KEYS was declared at module top level but never referenced. Remove it. - update_config had separate `value isa AbstractString && contains(key, '.')` and `elseif contains(key, '.')` branches that ran identical logic. Collapse to a single `contains(key, '.')` branch.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Promote each actionable finding from the codex/Copilot review cycle into a
contract test that targets the implicit specification, not just the
specific input that broke. Adds 73 tests across four files (692 → 765).
- parse_plugin_option_value: bool/int/decimal/bracket/quoted/unquoted
semantics, including the "1"/"0" stay-integer rule and the "Doe, Jane"
stay-string-when-quoted rule.
- ConfigCommand.execute: ArgParse-shaped nested args round-trip, repeated
--author becoming Vector{String}, --no-mise normalising to with_mise=false
(no leaked dashed keys), lowercase plugin keys mapping to canonical
PkgTemplates names, quoted plugin string values staying strings, mixed
flat dict (CLI key + dotted key) round-trip.
- CreateCommand.execute consume contract via dry-run: Vector author flows
as authors=Vector{String}, --mail appended only when author lacks <email>,
license_type promoted into License plugin (CLI License keeps precedence),
--with-mise / --no-mise CLI override the saved with_mise default.
- PackageGenerator.instantiate_plugins: Secret-typed fields wrap stored
strings, Union{Nothing,Secret} also wraps, String-typed fields are not
wrapped, License kwarg-only `name` is not over-coerced.
- ConfigManager: get_config_path/load_config/save_config honour
custom_path, create parent dirs as needed, and leave the XDG default
untouched.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
A direct caller passing only `config-file` to redirect the destination would have it written into `[default]` by `update_config`. Add it to the ignored-keys list alongside the meta-keys so the override never becomes a saved default. Adds a contract test exercising the legacy flat-dict path with `config-file` plus a dotted plugin key, and tightens the comment on the existing license-precedence test to record that it currently exercises execute()'s guard via the recognised "--License" key shape — the same guard via the bare ArgParse "license" key is blocked on issue #7.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…lity The plugin-flag handling listed Bool branches (true/false) as if some plugins were registered with :store_true, but config-set plugin options all use nargs='?' / constant='' / default=nothing, so value is only nothing / "" / String. Drop the dead Bool branches and rewrite the comment to describe the actual ArgParse contract.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ch create CreateCommand only treats nested Dicts whose key starts with an uppercase letter as plugin options, but update_config and _apply_set_args persisted dotted-section keys (`formatter.style`) under the literal lowercase `formatter`. Plugin defaults written via dot notation therefore never reached create. Resolve the section target case-insensitively: prefer an existing section (so manually edited lowercase TOML keeps being updated in place), and otherwise fall back to the canonical PkgTemplates name (`Formatter`) so new entries land under the form CreateCommand looks for. Update the existing dot-notation contract tests to assert the canonical form, and add a contract test asserting `[default.Formatter]` reaches create's plugin_options end-to-end.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
_resolve_section_target rebuilt the plugin-name lookup on every call, re-running PkgTemplates.concretes(Plugin) for each dotted key in a single config set. Compute the mapping once per update_config / _apply_set_args call and thread it through, so callers passing many dotted keys pay plugin discovery exactly once.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Three legal shapes are produced by ArgParse for `--<plugin>` under
`config set` (nargs='?', constant="", default=nothing): unspecified
(nothing), bare flag (""), and a KEY=VALUE bundle. The bundle case had
coverage; the other two relied on implicit behaviour. Add a contract
test so:
- nothing → no section is created (skip)
- "" → section appears empty (enable with defaults; create picks it up)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
jtc config setwas declared but never wired up — the ArgParse subcommand had no options andConfigCommand.executelooked at the wrong key (%SUBCOMMAND%vs ArgParse's%COMMAND%), so anyjtc config set --author Xreturnedunrecognized option. This PR ports the option surface and dispatch from the Python source (../JuliaPkgTemplatesCLI) soconfig set,config show, and the create flow that consumes those defaults all work end-to-end.Changes
config set/config showCLI surface: add--author(repeatable),--user,--mail,--license,--julia-version,--mise-filename-base, exclusive--with-mise/--no-mise,--config-file, plus per-plugin--git/--tests/etc. (skipping License to avoid colliding with--license).ConfigCommand.execute: read ArgParse's%COMMAND%, descend into nested sub-args, map options into the[default]section, and resolve plugin keys to PkgTemplates canonical names. Quoted strings stay literal (commas no longer auto-promote when the value was explicitly quoted).ConfigManager.{get_config_path,load_config,save_config}: optional custom path so--config-fileworks.CreateCommand.executenow consumes the new persisted shapes:default.license_typeis promoted into the License plugin section when no CLI license is supplied.--mail(now also acreateflag) appends<mail>to each author entry that doesn't already carry an email.--with-mise/--no-miseget normalized towith_miseso an explicit CLI flag overrides a saved default.PackageGenerator.instantiate_plugins: reflect on plugin field types and wrap stored strings intoPkgTemplates.Secretfor fields likeTagBot.token/TagBot.ssh/TagBot.gpg. PlainStringfields (e.g.License.name) are left untouched.Float64coercion that turnedversion=1.10into1.1and brokeProjectFile'sVersionNumberconversion (matches the Python port's int-only coercion viaisdigit).Out of scope
--tests "k=v,k=v"/--tests k=v --tests k=v/ help text) is a separate input-form normalization concern and will be tracked there.Test plan
Pkg.test()passes (692/692)jtc config set --author A --author B --user u --mail m --license MITwrites the expected TOML andjtc createproducesProject.tomlwithauthors = ["A <m>", "B <m>"]and a MIT LICENSEjtc config set --tagbot token=MY_TOKENfollowed byjtc createsucceeds (Secret wrap)jtc config set --projectfile version=1.10keeps the value as a stringconfig set --no-misethencreate --with-miseactually generates.mise.toml--git 'name="Doe, Jane"'savesGit.nameas a single string, not a listjtc config show --config-file <path>andjtc config set --config-file <path>round-trip through a custom location