chore: allow igniter global options (--yes, --dry-run, etc.) to pass through to sync task#55
Conversation
…through to sync task
There was a problem hiding this comment.
Pull request overview
Updates mix usage_rules.sync to stop rejecting all CLI args and instead allow Igniter’s global flags (e.g., --yes, --dry-run) while still rejecting unsupported arguments, so the task works properly when invoked via Igniter tooling.
Changes:
- Parse
argvagainst Igniter’s global option switch schema before deciding whether to raise. - Raise only when non-global args are present, and document supported Igniter flags in the warning output.
Comments suppressed due to low confidence (1)
lib/mix/tasks/usage_rules.sync.ex:120
- This change alters CLI behavior (allowing Igniter global flags and rejecting only certain args) but there are no tests covering
run/1argv parsing. Since there is already comprehensive test coverage for this task, please add tests that (1)mix usage_rules.sync --yes/--dry-run/--checkdoes not raise and (2) an unknown flag or positional argument still raises with the expected message.
def run(argv) do
{_opts, remaining, invalid} =
OptionParser.parse(argv, strict: Igniter.Mix.Task.Info.global_options()[:switches])
if Enum.any?(remaining ++ invalid) do
Mix.raise("""
WARNING: `mix usage_rules.sync` no longer accepts command-line arguments.
Configuration is now done in your `mix.exs` project config:
#{__MODULE__.Docs.code_sample(4)}
Then simply run: mix usage_rules.sync
Supported flags (passed through to Igniter): --yes, --dry-run, --verbose, --check
Run `mix help usage_rules.sync` for full configuration options.
""")
else
super(argv)
end
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {_opts, remaining, invalid} = | ||
| OptionParser.parse(argv, strict: Igniter.Mix.Task.Info.global_options()[:switches]) | ||
|
|
||
| if Enum.any?(remaining ++ invalid) do | ||
| Mix.raise(""" |
There was a problem hiding this comment.
invalid from OptionParser.parse/2 can include recognized switches with invalid values (e.g., wrong type). The PR description says only unrecognized arguments should be rejected, but this condition will also raise for invalid values of known Igniter global flags, bypassing Igniter's own (more precise) error handling. Consider only rejecting remaining and unknown switches, or letting invalid for known global flags pass through to super(argv) and be handled by Igniter.
lib/mix/tasks/usage_rules.sync.ex
Outdated
| @@ -108,6 +111,8 @@ if Code.ensure_loaded?(Igniter) do | |||
|
|
|||
| Then simply run: mix usage_rules.sync | |||
|
|
|||
| Supported flags (passed through to Igniter): --yes, --dry-run, --verbose, --check | |||
|
|
|||
| Run `mix help usage_rules.sync` for full configuration options. | |||
There was a problem hiding this comment.
The raised message says the task "no longer accepts command-line arguments" but then lists supported flags like --yes/--dry-run. This is internally inconsistent and may confuse users who passed an unsupported flag. Consider rewording to clarify that task-specific arguments are rejected but Igniter global flags are allowed (and keep the list of allowed flags separate from the warning).
|
The robot has some points (I think GH just turned this on for OSS PRs w/o asking? I'm not sure). But the main one is that we should clarify the wording slightly on the error message. Otherwise this makes perfect sense though and is a good catch, thank you 🙇 |
I’ve updated the wording of the error message based on Copilot’s review to make it clearer that task-specific arguments are rejected, while Igniter global flags are still allowed. I have Copilot set up to automatically review my PRs, so that’s why it showed up here. On a slightly unrelated note — from your experience, when using usage_rules, did you find it better to work with CLAUDE.md, or with skills? Thank you :) |
|
I've found skills to be less effective generally. |
|
🚀 Thank you for your contribution! 🚀 |
Contributor checklist
Leave anything that you believe does not apply unchecked.
Previously,
mix usage_rules.syncrejected all command-line arguments unconditionally. This was intended to guide users away from the old CLI-based configuration, but it also blocked Igniter's own global options like--yes.Now the task parses argv against Igniter's global option schema and only rejects unrecognized arguments. Igniter flags pass through as expected:
mix usage_rules.sync --yes— apply changes without confirmationmix usage_rules.sync --dry-run— preview changes onlymix usage_rules.sync --check— fail if changes are needed (useful in CI)