Skip to content

Port Packer, Gradle, jq, Liquibase and Ansible scrapers onto the shared wrapped-description helper - #4657

Merged
thomhurst merged 1 commit into
mainfrom
issue-4653-port-scrapers-group-a
Sep 6, 2026
Merged

Port Packer, Gradle, jq, Liquibase and Ansible scrapers onto the shared wrapped-description helper#4657
thomhurst merged 1 commit into
mainfrom
issue-4653-port-scrapers-group-a

Conversation

@thomhurst

Copy link
Copy Markdown
Owner

Summary

First child of #4646. Ports the five scrapers whose wrapped-description logic was closest to the shared shape onto CliScraperBase.AccumulateWrappedDescription (from #4645):

  • Packer — dropped the dash-led stop and the hand-rolled indent maths; a wrapped -only=foo,bar ... fragment now stays in the description instead of becoming a phantom option.
  • Gradle — dropped the absolute < 4 indent threshold; a wrapped --continue ... fragment stays prose.
  • jq — dropped the < 20 threshold; the displayed operands are stripped after the wrapped lines are joined, so the operand prefix is removed once.
  • Liquibase — dropped the < 20 threshold; the row predicate covers both the option and -D=PARAM shapes.
  • Ansible — line endings are normalised once at the split (no more per-line TrimEnd('\r')); sibling rows at the declaration's indentation now stop the description as they should.

Each scraper only supplies its own option-row predicate; none defines its own accumulation loop or indentation counter any more.

One base improvement fell out of the Liquibase port: a row whose prose only begins on the next line (picocli, argparse, git) had no description column, so the column-aware rule could not protect its later wrapped lines. AccumulateWrappedDescription now infers the column from the first wrapped line, which benefits every scraper.

Behavioural notes

Per the parent issue's scope note this changes generated output for these tools wherever wrapped descriptions previously mis-parsed; regenerate the five packages via the workflow after merge. Remaining scrapers are tracked in #4654 and #4655.

Test plan

  • New fixtures: Packer (new PackerCliScraperTests), Gradle, jq, Liquibase (inline and two-line rows), Ansible — each pins a wrapped --flag-led line as prose.
  • ModularPipelines.OptionsGenerator.Tests: 1304 passed, 0 failed (guarded local run).
  • dotnet format whitespace --verify-no-changes clean for the touched files (the only remaining hit, IHelpTextCache.cs, pre-exists on main).

Closes #4653

https://claude.ai/code/session_01PkLNTUfwGXjqXZrYrHaDGC

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-06T00:56:50.458852Z eee8705 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

…iption helper

Collapse the private AccumulateMultiLineDescription variants in the Packer,
Gradle, jq, Liquibase and Ansible scrapers onto
CliScraperBase.AccumulateWrappedDescription, so their absolute indent
thresholds and dash-led stop rules give way to the column-aware rule from
#4645: a wrapped line that looks like an option row stays prose when it starts
at or after the description column. Each scraper now only supplies its own
option-row predicate. jq strips its displayed operands after the wrapped lines
are joined; Ansible normalises line endings once at the split.

Teach the base to infer the description column from the first wrapped line
when a row carries no inline description, so two-line picocli and argparse
rows get the same protection.

Closes #4653

Claude-Session: https://claude.ai/code/session_01PkLNTUfwGXjqXZrYrHaDGC
@thomhurst
thomhurst force-pushed the issue-4653-port-scrapers-group-a branch from eee8705 to a32ef50 Compare September 6, 2026 00:52
@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 5be5e0ab-4b4b-43d3-a077-7cf7fd87984d

📥 Commits

Reviewing files that changed from the base of the PR and between 738bdf8 and a32ef50.

📒 Files selected for processing (11)
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/AnsibleCliScraperTests.cs
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/Cli/LiquibaseCliScraperTests.cs
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/GradleCliScraperTests.cs
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/JqCliScraperTests.cs
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/PackerCliScraperTests.cs
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/AnsibleCliScraper.cs
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/CliScraperBase.cs
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/GradleCliScraper.cs
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/JqCliScraper.cs
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/LiquibaseCliScraper.cs
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/PackerCliScraper.cs
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-4653-port-scrapers-group-a

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Code review

Reviewed the diff and the current state of pr-head (head a32ef5069b2b1f64031e9ed7e65c5d14f64f667d). No prior Claude review on this PR to reconcile against.

Summary: This ports Packer, Gradle, jq, Liquibase, and Ansible onto the shared CliScraperBase.AccumulateWrappedDescription helper (from #4645), removing each scraper's hand-rolled indent-threshold/dash-prefix continuation logic in favor of the column-aware rule. Each scraper now only supplies its own IsOptionRow predicate, matching the CLAUDE.md guidance to fix generator problems generically rather than per-tool.

Verified in detail:

  • No dangling references to the removed AccumulateMultiLineDescription methods remain in any of the five touched scrapers (confirmed via grep on pr-head); the untouched scrapers (Yarn, WinGet, Go, Az, DotNet, Pnpm, Pip, Terraform, Maven) correctly still use their own local helper, consistent with the PR's stated scope (tracked in Port Az, DotNet, Go, Maven, pip, pnpm, Terraform, WinGet and Yarn scrapers onto AccumulateWrappedDescription #4654/Use the shared continuation rule in CliScraperBase.HelpDeclaresRepeatableOption #4655).
  • Traced the new column-inference logic (descriptionColumn ??= GetIndentation(candidate); in CliScraperBase.cs) against IsContinuationLine: when a declaration has no inline description, the first wrapped line is accepted unconditionally (since a null descriptionColumn fails the >= comparison but !looksLikeOptionRow still lets it through for prose), after which the column is captured and used to correctly keep subsequent option-pattern-looking prose (e.g. a second wrapped --flag mention) attached to the description rather than mis-parsed as a new option row. Manually traced this against the Ansible and Gradle regex patterns and it lines up with the new test fixtures (e.g. --continue staying prose in the Gradle test, --null-input correctly excluded from jq's option list).
  • This base-method change also affects the already-migrated Cobra/Git/Hadolint/Cargo scrapers (not touched by this diff), which the PR description calls out explicitly as an intended, beneficial side effect — reasonable, since it only loosens an overly strict early-break rather than changing already-correct parsing.
  • New fixtures for each of the five scrapers look well targeted at the specific mis-parse each one used to have.

No actionable issues found — this is a clean, well-scoped refactor consistent with the repo's generated-options conventions.

@thomhurst
thomhurst merged commit 27ff415 into main Sep 6, 2026
16 checks passed
@thomhurst
thomhurst deleted the issue-4653-port-scrapers-group-a branch September 6, 2026 05:12
thomhurst added a commit that referenced this pull request Sep 6, 2026
…ion helper (#4658)

* refactor(generator): port nine more scrapers onto the shared wrapped-description helper

Collapse the private AccumulateMultiLineDescription variants in the Az,
DotNet, Go, Maven, pip, pnpm, Terraform, WinGet and Yarn scrapers (and
Go's private tab-aware indentation counter) onto
CliScraperBase.AccumulateWrappedDescription. Their dash-led stops, absolute
indent thresholds and ad hoc section-header heuristics give way to the
column-aware rule from #4645: a wrapped line that looks like an option row
stays prose when it starts at or after the description column, and prose
continues while it is deeper than the declaration. Each scraper now only
supplies its own option-row predicate; go joins the prose beneath its flag
lines because its inline text is derived rather than captured.

Teach the base to infer the description column from the first wrapped line
when a row carries no inline description (the same hunk ships in #4657), and
pin that mechanism with direct tests.

Closes #4654

Claude-Session: https://claude.ai/code/session_01PkLNTUfwGXjqXZrYrHaDGC

* fix(generator): accept the first wrapped line before a description column is known

With no inline description the column is inferred from the first wrapped
line, but IsContinuationLine compared against the still-null column, which
is always false in C#. A first wrapped line that also matched the scraper's
option pattern (any indented "-x ..." for Go) was therefore rejected before
the inference could run, and the main loop would mint a bogus option from
it. Treat an unknown column as "accept any line deeper than the
declaration"; later lines are still measured against the inferred column.

Refs #4654

Claude-Session: https://claude.ai/code/session_01PkLNTUfwGXjqXZrYrHaDGC
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port Packer, Gradle, jq, Liquibase and Ansible scrapers onto AccumulateWrappedDescription

1 participant