Repository navigation
feat(cli): add --imports/--exports/--symbols convenience flags - #209
mergify[bot] merged 7 commits into
Conversation
Add three boolean shorthands for the most common reverse-engineering tag filters: --imports (import), --exports (export), --symbols (demangled symbols). Each is behaviorally equivalent to the corresponding --only-tags value, conflicts with --only-tags and --raw at parse time, and composes with --no-tags via a resolved include set (Cli::resolved_include_tags). The runtime include/exclude overlap check and its error message are generalized to cover the resolved include set, not just --only-tags. Closes #208. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Move the resolved include-tag vec into with_include_tags instead of cloning it (the earlier overlap borrow is dead under NLL), and drop the inline comment that restated the helper's doc comment. No behavior change. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Move the --imports/--exports/--symbols tests into a dedicated integration_cli_convenience_flags.rs (keeps integration_cli.rs under the 500-line guideline). Run equivalence tests against test_binary_elf, which has non-zero import (81) and export (9) rows -- the previous PE fixture had zero exports, making the export/union assertions vacuous. Guard --symbols against aliasing --imports/--exports (no fixture has demangled rows), assert non-empty output where tags exist, and cover all six flag/conflict pairs. Sync the mdBook CLI reference with the three new flags. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds shorthand CLI tag-filter flags, resolves them into the effective include-tag set, switches tag rendering to canonical CLI names, and updates docs plus integration coverage for the new conflicts and overlap checks. ChangesConvenience flags implementation and validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Merge Protections🔴 1 of 4 protections blocking
🔴 🚦 Auto-queueThis rule is failing.When all merge protections are satisfied and these conditions match, this pull request will be queued automatically.
Show 3 satisfied protections🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 Full CI must passAll CI checks must pass. This protection prevents manual merges that bypass the merge queue.
🟢 Do not merge outdated PRsMake sure PRs are within 10 commits of the base branch before merging
|
There was a problem hiding this comment.
Pull request overview
This PR adds three convenience CLI flags to Stringy to make common reverse-engineering tag filters discoverable and ergonomic without requiring users to remember the exact Tag spellings.
Changes:
- Added
--imports,--exports, and--symbolsclap flags and a single resolved include-tag computation path used for both validation and filtering. - Generalized the runtime include-vs-exclude overlap validation message to cover both
--only-tagsand the new convenience flags. - Added integration tests and updated existing tests/docs to reflect the new flags and generalized error message.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/main.rs |
Adds the three convenience flags, resolves include tags in one place, and reuses that set for overlap validation and filtering. |
tests/integration_cli_convenience_flags.rs |
New integration coverage for equivalence, union behavior, clap conflicts, --no-tags composition, and --help surfacing. |
tests/integration_cli.rs |
Updates assertions to match the generalized overlap validation error message. |
tests/integration_cli_coverage.rs |
Updates validation error substring assertion to the generalized message. |
tests/integration_flow8_errors.rs |
Updates overlap error substring assertion to the generalized message. |
docs/src/cli.md |
Documents the new flags in the filtering options table. |
AGENTS.md |
Updates the CLI flags table to include the three new flags and the generalized overlap-check description. |
docs/plans/2026-07-04-001-feat-cli-convenience-flags-plan.md |
Adds an implementation-ready plan artifact capturing requirements, conflicts, and verification gates. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 67-70: Clarify the `--no-tags` entry in the CLI options table so
it matches runtime behavior: the convenience flags in the table (`--imports`,
`--exports`, `--symbols`) are not unconditionally compatible with `--no-tags`,
because overlap with the resolved include set still triggers a failure. Update
the documentation text near the `--no-tags` description to explicitly say
compatibility only holds when the excluded tags do not intersect the resolved
include set, and keep the wording aligned with the existing option names
(`--only-tags`, `--raw`, `--imports`, `--exports`, `--symbols`).
In `@tests/integration_cli_convenience_flags.rs`:
- Around line 133-140: The current convenience-flag compatibility test only
covers cli_imports_no_tags_compatible with --imports, so expand it to verify the
same non-overlap composition rule for the other shorthand flags as well. Update
the integration test in the convenience-flags suite to parameterize the success
case across cli_imports_no_tags_compatible, cli_exports_no_tags_compatible, and
cli_symbols_no_tags_compatible (or equivalent test coverage using
stringy().args(...) with --imports, --exports, and --symbols combined with
--no-tags). Keep the assertion behavior the same and ensure each convenience
flag is exercised with a safe non-overlapping --no-tags value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ceaca00e-0ad3-4223-ad1d-53aaeb567541
📒 Files selected for processing (8)
AGENTS.mddocs/plans/2026-07-04-001-feat-cli-convenience-flags-plan.mddocs/src/cli.mdsrc/main.rstests/integration_cli.rstests/integration_cli_convenience_flags.rstests/integration_cli_coverage.rstests/integration_flow8_errors.rs
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Queued — the merge queue status continues in this comment ↓. |
Add fixture-independent unit tests for Cli::resolved_include_tags() so the --symbols -> DemangledSymbol mapping (and the three-flag union) is falsifiable even though no fixture yields demangled-tagged rows. Strengthen the --help test to assert each flag's --only-tags shorthand line, and parameterize the --no-tags compatibility test across all three flags. Document --raw's full conflict set (now including the convenience flags) in AGENTS.md and the mdBook CLI reference, and clarify the resolved_include_tags doc that the convenience mechanism may combine several flags into a union. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/src/cli.md (1)
24-34: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSame "compatible" overstatement as AGENTS.md, not yet caught here.
Lines 31-33 describe
--imports/--exports/--symbolsas conflicting only with--only-tags/--raw, omitting that the runtime overlap check against--no-tagsstill fails when the excluded tag matches the resolved include set (e.g.,--imports --no-tags import, verified in tests/integration_cli_convenience_flags.rs lines 119-130). The--no-tagsrow itself (line 30) also doesn't mention this overlap-check behavior at all, unlike the equivalent AGENTS.md table update. Please align both rows with actual runtime behavior.As per path instructions, review documentation for accuracy with current implementation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/src/cli.md` around lines 24 - 34, The CLI docs table is overstating the compatibility of the convenience flags and omitting the `--no-tags` overlap behavior. Update the `--no-tags` row and the `--imports`/`--exports`/`--symbols` rows in `docs/src/cli.md` to match the actual runtime checks in the CLI parser, so they clearly state that these flags can still fail together when the resolved include set overlaps with excluded tags (for example, the convenience-flag handling in the same area as the `--only-tags`/`--raw` conflicts).Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/src/cli.md`:
- Around line 24-34: The CLI docs table is overstating the compatibility of the
convenience flags and omitting the `--no-tags` overlap behavior. Update the
`--no-tags` row and the `--imports`/`--exports`/`--symbols` rows in
`docs/src/cli.md` to match the actual runtime checks in the CLI parser, so they
clearly state that these flags can still fail together when the resolved include
set overlaps with excluded tags (for example, the convenience-flag handling in
the same area as the `--only-tags`/`--raw` conflicts).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b7832817-ebc9-4e5b-86bc-7768a1be7909
📒 Files selected for processing (4)
AGENTS.mddocs/src/cli.mdsrc/main.rstests/integration_cli_convenience_flags.rs
User-facing messages rendered tags with Debug (`Import`) instead of the CLI form users type (`import`): the --no-tags overlap error and the TTY summary's Top-tags line. Make Tag a clap ValueEnum so the canonical names live once in #[value(name = ...)] attributes; route FromStr through ValueEnum (single source) and add a Display impl that renders the canonical name via PossibleValue. Switch both Debug display sites to Display, and collapse the duplicate 25-arm tag_to_display_string helper (identical output) onto Display so the table, summary, error message, and parser all share one mapping. The arg parsers keep value_parser = <Tag as FromStr>::from_str, so --help and parsing behavior are unchanged; JSON serde serialization is untouched. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
|
@Mergifyio queue |
Merge Queue Status🛑 Queue command has been cancelled |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main.rs`:
- Line 106: The CLI args for Tag still use the custom FromStr parser instead of
clap’s native ValueEnum handling. Update the argument definitions in main.rs for
the --only-tags and --no-tags fields to use clap::value_parser!(Tag) or
#[arg(value_enum)] like the enc: Option<CliEncoding> field, so clap can provide
built-in value suggestions and consistent parsing. Make sure to adjust both
Tag-related args referenced in the same struct and verify tests don’t depend on
the old invalid-value error wording.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9345f73f-c21e-4a9b-b8c2-2f2920679af2
📒 Files selected for processing (6)
src/main.rssrc/output/table/formatting.rssrc/output/table/tty.rssrc/types/mod.rssrc/types/tests.rstests/integration_flows_6_7.rs
- main.rs: switch --only-tags/--no-tags to clap value_enum now that Tag derives ValueEnum (matches --enc); removes the hand-rolled from_str value_parser and the manual long_help tag list (clap now generates possible values). (CodeRabbit) - main.rs: sort + dedup the overlap error's tag list so repeated --only-tags values produce a stable message. (Copilot) - integration_flows_6_7.rs: tighten the TTY summary assertion to 'url:' so it can't match unrelated text. (Copilot) - AGENTS.md, docs/src/cli.md: qualify the --no-tags compatibility note -- the convenience flags still fail when an excluded tag overlaps the include set; document the overlap check on the --no-tags row. (CodeRabbit) - GOTCHAS.md: update the stale value_parser/from_str notes for value_enum. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main.rs`:
- Around line 188-190: The `tag_names` collection in `overlap` handling is doing
a sort-then-dedup on `Vec<String>`, which is correct but more verbose than
needed. Replace the `tag_names` vector construction with a `BTreeSet<String>` so
the sorted-unique intent is expressed directly and the extra `sort()` and
`dedup()` calls can be removed. Keep the surrounding `overlap.iter().map(|t|
t.to_string())` logic, but route it into the set-based collection instead of a
mutable vector.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 30775a47-f148-4bed-80b6-ceb82bd7019a
📒 Files selected for processing (5)
AGENTS.mdGOTCHAS.mddocs/src/cli.mdsrc/main.rstests/integration_flows_6_7.rs
- main.rs: collect the overlapping tag names into a BTreeSet<String> so they are sorted and de-duplicated in one pass, dropping the mutable Vec plus the separate sort()/dedup() calls. (CodeRabbit) Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Merge Queue Status
This pull request spent 13 minutes 49 seconds in the queue, including 13 minutes 32 seconds running CI. Required conditions to merge
|
Summary
Adds three boolean convenience flags --
--imports,--exports,--symbols--as shorthands for the most common reverse-engineering tag filters, so users don't
need to memorize exact
Tagspellings. Along the way it consolidates tag-namerendering onto a single clap
ValueEnumsource, fixing aDebugleak inuser-facing messages.
--imports--only-tags import--exports--only-tags export--symbols--only-tags demangledWhy
Imports, exports, and demangled symbols are already extracted, tagged, classified,
and ranked. They were filterable only via
--only-tags <TAG>, forcing users toknow the canonical spelling. The friction is worst for symbols: the tag is
demangled, but analysts reach for "symbols." These three queries ("what doesthis binary import / export / expose") are the highest-frequency RE tasks, so the
vocabulary friction lands on the most-used paths. The set is deliberately closed;
each flag's help points to
--only-tagsfor the full vocabulary.What changed
Source
src/main.rs-- three boolean clap fields; aCli::resolved_include_tags()helper that merges
--only-tagswith the convenience flags;run()rewired tocompute the resolved include set once (used for both the
--no-tagsoverlapcheck and the filter build). The overlap-validation error message is generalized
to describe an include/exclude contradiction rather than naming
--only-tags.src/types/mod.rs--Tagnow derivesclap::ValueEnum(canonical names liveonce in
#[value(name = ...)]);FromStrdelegates toValueEnum(singlesource of truth); a new
Displayimpl renders the CLI form viaPossibleValue.src/output/table/formatting.rs,src/output/table/tty.rs-- both tag-displaysites now use
Display; the duplicate 25-armtag_to_display_stringhelper isremoved. The table, summary, error message, and parser now share one mapping.
Tests
tests/integration_cli_convenience_flags.rs(new) -- equivalence, union,full 6-pair conflict matrix,
--no-tagscomposition, and--helpcoverage,against
test_binary_elf(import=81, export=9) so assertions are not vacuous.src/main.rs#[cfg(test)]unit tests -- fixture-independent checks that eachflag maps to the exact
Tag(the--symbols -> DemangledSymbolmapping can't beproven via fixtures, since none yield demangled rows).
src/types/tests.rs--Displayround-trip test over all variants.test updated for the corrected lowercase tag rendering.
Docs
AGENTS.mdanddocs/src/cli.md-- documented the three flags,--raw's fullconflict set, and the generalized
--no-tagsoverlap note.docs/plans/2026-07-04-001-feat-cli-convenience-flags-plan.md-- the plan doc.Type of change
--imports/--exports/--symbols)ValueEnum)Importinstead ofimport)Behavior and compatibility
--symbolsmaps toDemangledSymbolonly (all three flagsorthogonal -> clean union); flags conflict with
--only-tagsand--rawat parsetime (clap, bidirectional); compose with
--no-tagsvia the resolved include set.--no-tagsoverlap error and the TTY"Top tags" summary now render tags in the CLI form users type (
import) insteadof
Debug(Import). No machine-readable contract depended on the old text.variants) is a separate, tested contract and was intentionally not touched.
value_parser = <Tag as FromStr>::from_str(now backed byValueEnum), so--helpoutput and parsing behavior are byte-identical.How this was tested
cargo fmt --check,cargo clippy --all-targets -- -D warningsclean.tie-break ordering) against a fixture with real import/export rows.
--imports --no-tags import->conflicting tag filters: import are both included and excluded (--no-tags).(Linux/macOS/Windows) were green on the prior head; re-running on the latest commit.
Review guidance (commit-by-commit)
The five commits are independently reviewable in order:
feat(cli)-- flags +resolved_include_tags+run()rewire + tests + docs.refactor(cli)-- drop a needless clone (move intowith_include_tags).test(cli)-- split convenience tests into their own file; fix vacuous fixtures.test(cli)-- fixture-independent mapping unit tests; complete conflict docs.refactor(tags)--TagasValueEnum; single-source Display; remove theduplicate mapping helper.
The highest-leverage file to review is
src/types/mod.rs(theValueEnum+FromStr+Displaywiring); everything else follows from it.Review checklist
General
dbg!/println!left in library codeunsafe; no blanket#[allow]Code quality
resolved_include_tags()is the single authority for the include setconflicts_with_allcovers--only-tagsand--rawfor all three flagsValueEnumnames ->FromStr+Display)Testing
--symbols -> DemangledSymbolis pinned by a fixture-independent unit testDocs
--rawconflict set is complete in both docsRisk assessment
Overall: Low.
derivefeature--help/parsing unchanged; one non-breaking UX text change