fix(completion): rely on usage#649 for global flags, keep -r/-S promotion#10176
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughSelective completion-spec promotion was implemented: only mounted flags whose long matches a root global and that expose short aliases absent from the root global are marked global. The usage spec minimum version was raised to 3.4, global attributes removed from run command flags, and e2e tests extended to cover orphan-short flags ( ChangesOrphan short flag promotion for completion specs
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR narrows the completion-spec workaround for
Confidence Score: 5/5Safe to merge — a well-scoped narrowing of an existing workaround with consistent updates to both the static KDL and runtime code. The promote_orphan_shorts logic correctly targets only -r/-S, the min_usage_version bump guards against older parsers, and the new e2e assertions directly exercise the fixed paths. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "fix(completion): rely on usage#649 for g..." | Re-trigger Greptile |
…tion The `run`/`tasks run` subcommands redeclare several root globals as their own non-global flags. When the usage parser descends into the mounted task spec it keeps only inherited globals, so a flag placed before the task used to be dropped and mis-parsed during completion (mise#10069). mise#10153 worked around this by promoting every colliding flag (-C/-j/-q/-r/-S) to global in the completion spec. usage#649 (in usage 3.4.0) fixes the common case in the parser: an inherited global redeclared as non-global is now preserved together with all its aliases. That covers -C/--cd, -j/--jobs and -q/--quiet, whose short is itself a root global short, so they no longer need the workaround. It cannot cover -r/--raw and -S/--silent: their root globals are long-only (--raw/--silent have no short), and the -r/-S shorts exist only on the non-global `run` redeclarations, which the parser drops. So narrow the old "promote everything" closure to just these orphan-short flags (long matches a root global, short does not). - src/cli/usage.rs: replace the broad promote with promote_orphan_shorts, and set min_usage_version to "3.4" so usage CLIs older than the #649 fix warn to upgrade instead of silently re-triggering mise#10069. - mise.usage.kdl: -C/-j/-q drop global=#true (now handled upstream); -r/-S keep it. usage-lib itself is left at 3 -- spec generation is unaffected by #649 and the runtime requirement is carried by min_usage_version (the e2e installs usage latest). - e2e: add -r/-S coverage over the run and tasks-run paths. Verified locally with usage 3.4.0: completion returns the task's choices for -C/-j/-q/-r/-S before the mounted task. Purely-run flags with no root-global counterpart (-f/-n/-o) remain a separate pre-existing completion limitation, unchanged here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
c99ea84 to
e70c600
Compare
Problem
Completion fails when a
run/tasks runflag (or a root global flag) precedes the task, e.g.:The
run/tasks runsubcommands redeclare several root globals as their own non-global flags. When the usage parser descends into the mounted task spec it keeps only inherited globals, so a flag placed before the task was dropped and mis-parsed during completion (#10069). #10153 worked around this by promoting every colliding flag (-C/-j/-q/-r/-S) toglobalin the completion spec.What changed
jdx/usage#649 (released in usage 3.4.0, see jdx/usage#652) fixes the common case in the parser: an inherited global redeclared as non-global is now preserved together with all its aliases. That covers
-C/--cd,-j/--jobsand-q/--quiet, whose short is a root global short, so they no longer need the workaround.It cannot cover
-r/--rawand-S/--silent: their root globals are long-only (--raw/--silenthave no short), and the-r/-Sshorts exist only on the non-globalrunredeclarations, which the parser drops. So this PR narrows the old "promote everything" closure to just these orphan-short flags (long matches a root global, short does not).src/cli/usage.rs: replace the broad promote withpromote_orphan_shorts, and setmin_usage_versionto"3.4"so usage CLIs older than the #649 fix warn to upgrade instead of silently re-triggering mise completion bug with `-C` and task argument `choices` #10069.mise.usage.kdl:-C/-j/-qdropglobal=#true(now handled upstream by #649);-r/-Skeep it.min_usage_versionis bumped to3.4. (Regenerated on top of fix(task): bump usage for completions #10181, so the surrounding spec is already in the 3.4.0 format — this PR's diff is limited to theglobalattribute and the version line.)e2e/tasks/test_task_completion_global_cd: add-r/-Scoverage over therunandtasks runpaths, using the tab-separated completion output introduced in fix(task): bump usage for completions #10181.Verification
Locally with usage 3.4.0, completion returns the task's choices for
-C/-j/-q/-r/-Sbefore the mounted task:cargo fmt,shfmt,shellcheckare clean. Shell completions, man pages, fig, and docs are unaffected (theglobalattribute is not rendered into those artifacts).Out of scope
Purely-
runflags with no root-global counterpart (-f/-n/-o) remain a separate pre-existing completion limitation when placed before the mounted task; not introduced or changed by this PR.Summary by CodeRabbit
Bug Fixes
runandtasks run, including edge cases where global flags appear before the command and when short-form flags are redeclared; orphan short flags (-r/--raw,-S/--silent) are now correctly recognized and suggested.Chores