Skip to content

refactor: extend for-comprehension usage beyond argparse#3445

Closed
bobzhang wants to merge 1 commit intomainfrom
hongbo/comprehension-batch
Closed

refactor: extend for-comprehension usage beyond argparse#3445
bobzhang wants to merge 1 commit intomainfrom
hongbo/comprehension-batch

Conversation

@bobzhang
Copy link
Copy Markdown
Contributor

@bobzhang bobzhang commented Apr 20, 2026

Summary

Follow-up to #3444. Apply the same for-comprehension pattern to other imperative loops that just accumulate into an array:

  • `argparse/help_render.mbt`: `positional_entries`, `subcommand_entries`, `group_members`
  • `priority_queue`: `ToJson` implementation
  • `bytes/internal/regex_engine/translate.mbt`: `Alternation` arm (uses the `is (cr, pref2)` pattern to destructure inline)

Skipped candidates where the comprehension would lose an explicit `capacity` pre-alloc hint or where the loop has branching pushes (e.g., `arg_display`, `option_entries`, `positional_usage`, `List::to_json`).

Test plan

  • `moon check` clean
  • `moon test -p argparse -p priority_queue -p bytes` passes (453 tests)
  • `moon fmt --check` clean
  • `codex review --uncommitted` confirms behavior-preserving

🤖 Generated with Claude Code


Open in Devin Review

Apply the same for-comprehension pattern used in required_option_usage
to other places where an imperative loop accumulates into an array:

- argparse/help_render.mbt: positional_entries, subcommand_entries, group_members
- priority_queue: ToJson implementation
- bytes regex_engine translate: Alternation arm

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@coveralls
Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 3874

Coverage decreased (-0.003%) to 94.911%

Details

  • Coverage decreased (-0.003%) from the base build.
  • Patch coverage: 13 of 13 lines across 3 files are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 15544
Covered Lines: 14753
Line Coverage: 94.91%
Coverage Strength: 211421.39 hits per line

💛 - Coveralls

@bobzhang
Copy link
Copy Markdown
Contributor Author

Superseded by #3448 which consolidates all comprehension refactors.

@bobzhang bobzhang closed this Apr 21, 2026
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.

2 participants