fix: fix feature = "cargo-clippy" deprecation#1626
Merged
Conversation
jqnatividad
added a commit
that referenced
this pull request
Apr 23, 2026
…handle continuations Addresses roborev review #1626: 1. Options-section scope: replace the blank-line-after-Usage heuristic with a proper line-anchored, case-insensitive `options:` / `Options:` header regex. The previous logic landed on description paragraphs (most qsv USAGEs have a description between the Usage block and the options section), so the pair scan was broader than intended. 2. Fallback for minimal fixtures: if no options header is found, fall back to scanning the whole USAGE so short↔long pairs declared in non-standard layouts (or small test fixtures) still register. 3. Compiled regexes (short-first pair, long-first pair, flag scanner, options header) are now cached via `std::sync::OnceLock`, removing per-call recompilation overhead. 4. Usage-block collection now terminates only on a blank line and merges docopt continuation lines (indented lines within the block that do not begin with `qsv`) into their parent variant, preventing a wrapped Usage line from showing up as a standalone variant and silently narrowing the intersection. Adds three more unit tests covering: continuation-line joining (`continuation_line_does_not_truncate_usage_block`), scope narrowing (`pair_regex_scans_only_the_options_section_not_description`), and the whole-text fallback (`fallback_to_whole_text_when_no_options_section`). 15/15 unit tests pass; regenerator output is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jqnatividad
added a commit
that referenced
this pull request
Apr 23, 2026
…ls (#3734) * feat(generators): detect required options from Usage: line Both help_markdown_gen.rs and mcp_skills_gen.rs now identify options shown outside [options]/[...] groups in the USAGE's `Usage:` section (e.g. `qsv implode [options] -k <keys> -v <value>`) and mark them accordingly. - `docs/help/*.md`: required options get ` **(required)**` appended to their description column in the options table. - `.claude/skills/qsv/*.json`: option entries gain `"required": true` when the flag is required. Optional options continue to emit nothing (the field is skipped when absent). A small wrinkle worth noting: qsv-docopt's Parser does not always emit Atom::Short entries paired with the Long atom for the `-k, --keys` declaration style, so we can't rely on its pairing to expand short↔long forms. Both generators do their own pairing pass by scanning the options sections for `-X, --xxx` declarations. Closes roborev review #1618 findings #3 and #4 at a project-wide (generator) level. Findings #1 (empty positional arg descriptions) and #2 (unfenced CSV examples) remain as separate work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: regenerate help markdown and MCP skills for required-option markers Commands with required options in their Usage: line now show them: - applydp: --new-column, --replacement, --formatstr - apply, describegpt, fetchpost, implode, joinp, luau, py, split: various required options previously unmarked All regenerated via `qsv --generate-help-md` and `qsv --update-mcp-skills`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(generators): only mark an option required when it's required in all Usage variants The previous heuristic produced seven false positives flagged by roborev review #1624. The detector now: 1. Computes a per-Usage-variant required set (tokens outside `[...]` AND outside any `(A | B)` alternative group), then takes the intersection across all non-`--help` Usage lines. An option must be required in every variant to be marked globally required. 2. Handles `(A | B | C)` alternative groups by masking them out entirely — inside alternatives no individual token is required. 3. Expands short→long aliases per Usage line, before intersection, so `-n`'s use as a positional-style flag on one Usage variant doesn't leak into `--no-headers` as globally required. Fixes false-positive required markers on: - split: `--size` / `--chunks` / `--kb-size` (alternative group) - joinp: `--cross` / `--non-equi` (separate Usage lines) - apply / applydp: `--new-column` / `--replacement` / `--formatstr` (subcommand-scoped, not global) - describegpt: `--prepare-context` / `--process-response` (separate Usage lines) - fetchpost: `--payload-tpl` (alternative inside `(A | B)`) - luau / py: `--no-headers` (the `-n <main-script>` Usage role only appears in one variant; intersection excludes it) - py: `--helper` (only required on one Usage variant) Implode's `--keys` / `--value` markers are preserved (genuinely required in the single Usage variant). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(generators): share required-options detection in a common module Addresses roborev review #1625. Extracts the previously-duplicated required-option detection (and its helpers) out of both generators and into a new crate::generators_common module. Both mcp_skills_gen and help_markdown_gen now delegate to it, keeping their detection semantics in lockstep. Fixes additional review findings while consolidating: - Bidirectional short↔long expansion via a new FlagPairs type, so Usage lines that mention only the long form also surface the short form in the required set (and vice versa). - Bracket-depth is now u32, and uses u32::saturating_sub so an unbalanced `]` cannot underflow the counter (on i32 it would have saturated at i32::MIN, silently dropping later required tokens). - The pair regex now matches long-first (`--keys, -k`) declarations in addition to short-first (`-k, --keys`). - The pair regex scans only the options-declaration portion of the USAGE string (after the Usage: block), so a future quirk in the Usage: block can't introduce a bogus pair. Adds 12 unit tests covering: single-variant required expansion, alternative groups `(A|B|C)`, multi-variant intersection, subcommand- scoped options, short-role collision (luau/py), plain `(X)` grouping without a pipe, nested optional inside an alt group, long-first declarations, long-only Usage mentions, no Usage block, unbalanced brackets, and Usage-block scoping of the pair regex. Regenerator outputs (docs/help and .claude/skills) are unchanged by this refactor (confirmed with --generate-help-md / --update-mcp-skills producing no diffs). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(generators_common): narrow options-section scope, cache regexes, handle continuations Addresses roborev review #1626: 1. Options-section scope: replace the blank-line-after-Usage heuristic with a proper line-anchored, case-insensitive `options:` / `Options:` header regex. The previous logic landed on description paragraphs (most qsv USAGEs have a description between the Usage block and the options section), so the pair scan was broader than intended. 2. Fallback for minimal fixtures: if no options header is found, fall back to scanning the whole USAGE so short↔long pairs declared in non-standard layouts (or small test fixtures) still register. 3. Compiled regexes (short-first pair, long-first pair, flag scanner, options header) are now cached via `std::sync::OnceLock`, removing per-call recompilation overhead. 4. Usage-block collection now terminates only on a blank line and merges docopt continuation lines (indented lines within the block that do not begin with `qsv`) into their parent variant, preventing a wrapped Usage line from showing up as a standalone variant and silently narrowing the intersection. Adds three more unit tests covering: continuation-line joining (`continuation_line_does_not_truncate_usage_block`), scope narrowing (`pair_regex_scans_only_the_options_section_not_description`), and the whole-text fallback (`fallback_to_whole_text_when_no_options_section`). 15/15 unit tests pass; regenerator output is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(generators_common): tighten options-header regex and indentation-aware continuation Addresses roborev review #1627: 1. options_section regex: the leading class is now `[ \t\w-]` instead of `[\s\w-]`, so it can't straddle a newline. Trailing `[ \t]*` matches only the same-line whitespace, keeping the "line-anchored" claim in the doc comment literally true. 2. collect_usage_lines: drop the dead `if trimmed.is_empty()` branch — the `take_while(!blank)` already filters blank lines out. 3. collect_usage_lines: continuation detection now prefers indentation depth (leading-whitespace count strictly greater than the parent variant's) with the `qsv`-prefix rule as a tiebreaker. A continuation line whose positional begins with `qsv` (e.g. `qsv-input`) would previously have been treated as a new variant. 4. collect_usage_lines: inline `Usage: qsv foo ...` header variants are now retained. Previously the `Usage:` line was unconditionally dropped, silently losing the only variant for that style of help text. Three additional unit tests lock this in: `Common options:` / `map options:` prefix-word headers both matched; a tab-indented `\toptions:` header; and an inline `Usage: qsv foo ...` variant. 18/18 unit tests pass; full `cargo test` passes; generator outputs unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(generators_common): baseline-indent continuation detection + cleanups Addresses roborev review #1628: 1. Indentation comparison now uses a single *baseline indent* (the leading whitespace of the first non-blank line in the Usage block) instead of comparing each line against the previous variant's indent. This fixes the inline-vs-non-inline asymmetry: an inline `Usage: qsv foo --bar` header was stored trimmed (leading_ws=0), so a following standard- column variant like ` qsv foo --baz` (leading_ws=7) was wrongly merged as a continuation. Inline variants are now synthesized at the baseline indent for consistent comparison. 2. With the baseline-indent rule, indentation genuinely *outranks* any prefix test: continuation == leading_ws > baseline. No more confused comment claiming "tiebreaker" while the code actually OR'd. The `qsv`-prefix check is gone — indentation is the sole signal. 3. `if let Some(last) = variants.last_mut()` replaces the `match + later unwrap()` pattern, removing the SAFETY comment. Three new unit tests lock this in: - `indented_wrap_line_merges_into_parent_variant` - `continuation_starting_with_qsv_prefix_is_still_a_continuation` (a deeper-indented `qsv-foo` continuation must fold, regression for the indentation-outranks-prefix intent) - `inline_usage_plus_indented_second_variant_stays_separate` (regression for the storage asymmetry from finding #1) 21/21 unit tests pass; generator outputs unchanged; clippy clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(generators_common): derive baseline indent from min leading-ws Addresses roborev review #1629: 1. Baseline-indent is now the *minimum* leading whitespace across all non-blank lines in the Usage block (not the leading_ws of raw[0]). In well-formed docopt, continuation lines are always indented deeper than their variants, so min reliably picks the variant column — including when raw[0] happens to be a wrapped continuation of an inline `Usage:` variant (previously baseline would have drifted to the continuation's indent, causing real variants at the standard column to be misclassified). 2. The continuation branch now fails loudly (debug_assert) when hit with an empty variants vec rather than silently promoting the line, so a future refactor of the baseline derivation can't regress unnoticed. 3. Replaced `.map().next().unwrap_or(0)` with `.iter().map(...).min()` — both a cleaner expression and the fix for the baseline-drift bug. 4. Added `inline_usage_with_wrapped_continuation_and_second_variant` to lock in the exact corner case: inline `Usage: qsv foo --bar`, an indented wrap line, and a second variant at the standard column — baseline must land on the standard column, the wrap line must merge into variant 1, and the second variant must stay separate. 5. Doc-comment reworded to describe the actual invariant: "at or below the baseline indent start new variants; strictly deeper lines are merged." 22/22 unit tests pass; generator outputs unchanged; clippy clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(skills): add optional Option.required to TypeScript schema The Rust generator emits `"required": true` on required options in the MCP skill JSON (via the newly-added Option_.required field in mcp_skills_gen.rs). The TypeScript Option interface didn't declare it, so consumers using the typed view wouldn't see the field even though the JSON carries it. Adds `required?: boolean` with a doc comment matching the generator's semantics: emitted only when true, omitted for optional options. Addresses Copilot review on PR #3734. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
2 participants
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.
https://blog.rust-lang.org/2024/02/28/Clippy-deprecating-feature-cargo-clippy.html