docs: Improve README, CLI UX, and documentation accuracy#147
docs: Improve README, CLI UX, and documentation accuracy#147unclesp1d3r merged 19 commits intomainfrom
Conversation
…-approval Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…and usage sections Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…ne run Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…ormatting Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
… CLI documentation Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…classifications and examples Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
📝 WalkthroughArchitectural ImpactTrait-based Encoding Conversion: Refactored CLI encoding argument handling by introducing a Diagnostic Signal Enhancement: Updated the "no strings matched" diagnostic to provide actionable guidance (adjusting API Changes (Breaking)Removed EXIT CODES Standardization: Expanded and corrected exit code scheme from 2-code to 4-code model:
Explicit documentation added to Binary Format and Semantic Classification Improvementsstdin Support with Format Diversity: Comprehensive test coverage added (6 integration tests in Semantic Tag Completeness: Expanded semantic tag set from 17 to 21 tags by adding Documentation Corrections (Accuracy)Removed Fictional Content: Corrected stale documentation by eliminating 20+ fictional CLI flags, fictional environment variables, fictional config formats, and a non-existent Docker section. MSRV Correction: Updated minimum supported Rust version from 1.70 to 1.91 in installation documentation. Performance Documentation Rewrite: Replaced 355 lines of vague performance metrics with targeted prose describing the single-threaded sequential pipeline, Big-O complexity phases, and practical CLI narrowing strategies for optimization. Testing AdditionsAdded 7 new integration tests targeting stdin handling (PE, Mach-O, ELF, JSON output, raw mode, filtered mode), exit code validation, and diagnostic message accuracy. All 617 existing tests pass; clippy and fmt checks clean. Configuration ToolingMergify configuration simplified from complex label/author/workflow detection to three streamlined queues (dosubot, dependabot, default) with unified auto-approval rules for bot PRs. Tessl MCP server configuration added across multiple IDE/tool config files (.cursor/mcp.json, .gemini/settings.json, .mcp.json, .codex/config.toml) for AI-assisted development support. WalkthroughAdds tessl MCP server entries across multiple config formats, removes the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 📃 Configuration Change RequirementsWonderful, this rule succeeded.Mergify configuration change
🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 CI must passWonderful, this rule succeeded.All CI checks must pass. This protection prevents manual merges that bypass the merge queue.
🟢 Do not merge outdated PRsWonderful, this rule succeeded.Make sure PRs are within 10 commits of the base branch before merging
|
coderabbitai
bot
added
documentation
There was a problem hiding this comment.
Pull request overview
Improves Stringy user-facing documentation and CLI ergonomics, aligning help text and docs with current behavior while adding integration tests for stdin and flag UX.
Changes:
- Updates CLI help/error UX (exit codes in
--help,--enclong-only, improved diagnostics) and adds stdin-focused integration tests. - Rewrites/refreshes docs (installation, performance, CLI reference, quickstart tags) and modernizes README structure/badges.
- Simplifies Mergify configuration into bot-specific queues plus a default queue.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integration_cli_stdin.rs | Adds stdin edge-case integration tests for multiple input types and modes. |
| tests/integration_cli_short_flags.rs | Removes -e coverage and updates short-flag tests around --enc. |
| tests/integration_cli.rs | Verifies --help includes an EXIT CODES: section. |
| tessl.json | Adds Tessl configuration (vendored skill dependency list). |
| src/types/error.rs | Adds recovery hints to selected error messages. |
| src/pipeline/mod.rs | Improves no-filter-match diagnostic guidance. |
| src/main.rs | Removes -e, adds exit codes to help, and refactors encoding mapping via From. |
| docs/src/quickstart.md | Completes the semantic tag table with additional tags/examples. |
| docs/src/performance.md | Replaces stale/fictional performance docs with current behavior and real commands. |
| docs/src/installation.md | Updates install guidance (MSRV, releases, removes fictional Docker section). |
| docs/src/cli.md | Aligns documented exit codes with StringyError::exit_code() and clarifies clap behavior. |
| README.md | Restructures README with badges, quickstart, security notes, and link references. |
| GOTCHAS.md | Updates CLI short-flag notes and clap derive limitations guidance. |
| AGENTS.md | Updates CLI flag matrix to reflect --enc long-only. |
| .mergify.yml | Simplifies queue rules and auto-approval for bots; adjusts protections. |
| .mcp.json | Adds MCP server config for Tessl. |
| .gitignore | Ignores Tessl-generated artifacts. |
| .gemini/settings.json | Adds Tessl MCP server config for Gemini tooling. |
| .cursor/mcp.json | Adds Tessl MCP server config for Cursor tooling. |
| .codex/config.toml | Adds Tessl MCP server config for Codex tooling. |
tests/integration_cli_stdin.rs
Outdated
| stringy() | ||
| .arg("-") | ||
| .write_stdin(fixture_data) | ||
| .assert() | ||
| .success() | ||
| .stdout(predicate::str::is_empty().not()); |
| - name: Full CI must pass | ||
| description: All CI checks must pass. This protection prevents manual merges that bypass the merge queue. | ||
| if: | ||
| - base = main |
| .arg("10") | ||
| .arg("-t") | ||
| .arg("5") | ||
| .arg("-e") | ||
| .arg("--enc") | ||
| .arg("ascii") |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.mergify.yml:
- Around line 2-10: The dosubot mergify rule named "dosubot" currently allows
merging when only the "quality" check passes; update the rule to also require
the "test" check (or another full CI gate) before merge by modifying the
merge_conditions for the dosubot rule so it includes "- check-success = test" in
addition to "- check-success = quality", ensuring substantive code changes get
the full CI matrix validation.
In `@docs/src/installation.md`:
- Around line 46-49: The fenced code block containing the cargo commands is
missing a language specifier; update the opening fence from ``` to ```bash so it
reads ```bash followed by the lines "cargo build --release" and "cargo test" and
the closing ``` to satisfy the MD040 rule and enable proper syntax highlighting
in the docs.
In `@src/pipeline/mod.rs`:
- Around line 111-114: The multi-line string literal starting with "Info: No
strings matched the current filters" in src/pipeline/mod.rs is using backslash
line-continuation which preserves the next line's indentation and injects
unwanted spaces; change it to a single uninterrupted string or concatenate two
string literals without leading whitespace (or use format!/format_args! / raw
string) so the "{total_count} extracted, 0 shown" and the "Try removing ..."
sentence are printed without the extra indentation; locate the literal text and
update it (the string containing "{total_count} extracted, 0 shown" and the
following "Try removing --only-tags..." clause) to remove the embedded leading
spaces.
In `@tests/integration_cli_short_flags.rs`:
- Around line 103-114: Add a negative test to ensure the removed short flag `-e`
is rejected: create a new test (e.g., fn test_enc_short_flag_rejected()) that
uses the same helper stringy() and the same elf_path
("tests/fixtures/test_binary_elf") but passes "-e" "ascii" instead of "--enc",
then assert that the command fails (non-success) and/or returns a specific error
code or stderr message; reference the existing test function
test_enc_long_flag_only and the stringy() helper so maintainers can locate where
to add the new failing assertion.
In `@tests/integration_cli_stdin.rs`:
- Around line 13-14: Replace relative fixture reads in
tests/integration_cli_stdin.rs with manifest-rooted paths using the
CARGO_MANIFEST_DIR env macro: locate each std::fs::read call that currently uses
"tests/fixtures/..." (e.g., the fixture_data binding and the repeated reads at
the other mentioned locations) and construct the path by joining
env!("CARGO_MANIFEST_DIR") with "tests/fixtures/<filename>" before calling
std::fs::read so the tests no longer depend on the current working directory.
🪄 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: d291dac0-44da-43ac-b9f4-5f27148b2849
📒 Files selected for processing (20)
.codex/config.toml.cursor/mcp.json.gemini/settings.json.gitignore.mcp.json.mergify.ymlAGENTS.mdGOTCHAS.mdREADME.mddocs/src/cli.mddocs/src/installation.mddocs/src/performance.mddocs/src/quickstart.mdsrc/main.rssrc/pipeline/mod.rssrc/types/error.rstessl.jsontests/integration_cli.rstests/integration_cli_short_flags.rstests/integration_cli_stdin.rs
| ```bash | ||
| xcode-select --install | ||
| ``` | ||
|
|
||
| ### Windows | ||
|
|
||
| Install Visual Studio Build Tools or Visual Studio Community with C++ support. | ||
|
|
||
| Alternatively, use the GNU toolchain: | ||
|
|
||
| ```bash | ||
| rustup toolchain install stable-x86_64-pc-windows-gnu | ||
| rustup default stable-x86_64-pc-windows-gnu | ||
| ``` | ||
|
|
||
| ## Docker (Alternative) | ||
|
|
||
| If you prefer containerized builds: | ||
|
|
||
| ```dockerfile | ||
| FROM rust:1.70 as builder | ||
| WORKDIR /app | ||
| COPY . . | ||
| RUN cargo build --release | ||
|
|
||
| FROM debian:bookworm-slim | ||
| RUN apt-get update && apt-get install -y ca-certificates && rm -rf /var/lib/apt/lists/* | ||
| COPY --from=builder /app/target/release/stringy /usr/local/bin/ | ||
| ENTRYPOINT ["stringy"] | ||
| ``` | ||
|
|
||
| Build and run: | ||
|
|
||
| ```bash | ||
| docker build -t stringy . | ||
| docker run --rm -v $(pwd):/data stringy /data/binary_file | ||
| cargo build --release | ||
| cargo test | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
Static analysis flagged this code block as missing a language identifier (MD040). Adding bash improves syntax highlighting and accessibility.
Proposed fix
-```
+```bash
cargo build --release
cargo test
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/src/installation.md` around lines 46 - 49, The fenced code block
containing the cargo commands is missing a language specifier; update the
opening fence from ``` to ```bash so it reads ```bash followed by the lines
"cargo build --release" and "cargo test" and the closing ``` to satisfy the
MD040 rule and enable proper syntax highlighting in the docs.
| "Info: No strings matched the current filters \ | ||
| ({total_count} extracted, 0 shown)\n \ | ||
| Try removing --only-tags, --no-tags, or --enc filters to see more results" | ||
| ); |
There was a problem hiding this comment.
String continuation includes leading whitespace, producing malformed output.
The backslash line continuation in Rust string literals includes all leading whitespace from the next line. This produces output with excessive spaces:
Info: No strings matched the current filters (5 extracted, 0 shown)
Proposed fix
if filtered.is_empty() && total_count > 0 {
eprintln!(
- "Info: No strings matched the current filters \
- ({total_count} extracted, 0 shown)\n \
- Try removing --only-tags, --no-tags, or --enc filters to see more results"
+ "Info: No strings matched the current filters ({total_count} extracted, 0 shown)\n \
+Try removing --only-tags, --no-tags, or --enc filters to see more results"
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pipeline/mod.rs` around lines 111 - 114, The multi-line string literal
starting with "Info: No strings matched the current filters" in
src/pipeline/mod.rs is using backslash line-continuation which preserves the
next line's indentation and injects unwanted spaces; change it to a single
uninterrupted string or concatenate two string literals without leading
whitespace (or use format!/format_args! / raw string) so the "{total_count}
extracted, 0 shown" and the "Try removing ..." sentence are printed without the
extra indentation; locate the literal text and update it (the string containing
"{total_count} extracted, 0 shown" and the following "Try removing
--only-tags..." clause) to remove the embedded leading spaces.
| let fixture_data = | ||
| std::fs::read("tests/fixtures/test_binary_pe.exe").expect("PE fixture should exist"); |
There was a problem hiding this comment.
Use manifest-rooted fixture paths to avoid cwd-dependent test failures.
Line 14 and the repeated fixture reads rely on relative paths (tests/fixtures/...). This is brittle outside the default cargo test working directory.
Proposed fix
+use std::path::PathBuf;
use assert_cmd::Command;
use assert_cmd::cargo_bin_cmd;
use predicates::prelude::*;
fn stringy() -> Command {
cargo_bin_cmd!("stringy")
}
+
+fn fixture_path(name: &str) -> PathBuf {
+ PathBuf::from(env!("CARGO_MANIFEST_DIR"))
+ .join("tests")
+ .join("fixtures")
+ .join(name)
+}
+
+fn read_fixture(name: &str, label: &str) -> Vec<u8> {
+ std::fs::read(fixture_path(name)).expect(label)
+}
@@
- let fixture_data =
- std::fs::read("tests/fixtures/test_binary_pe.exe").expect("PE fixture should exist");
+ let fixture_data = read_fixture("test_binary_pe.exe", "PE fixture should exist");
@@
- let fixture_data =
- std::fs::read("tests/fixtures/test_binary_macho").expect("Mach-O fixture should exist");
+ let fixture_data = read_fixture("test_binary_macho", "Mach-O fixture should exist");
@@
- let fixture_data =
- std::fs::read("tests/fixtures/test_unknown.bin").expect("unknown fixture should exist");
+ let fixture_data = read_fixture("test_unknown.bin", "unknown fixture should exist");
@@
- let fixture_data =
- std::fs::read("tests/fixtures/test_binary_elf").expect("ELF fixture should exist");
+ let fixture_data = read_fixture("test_binary_elf", "ELF fixture should exist");
@@
- let fixture_data =
- std::fs::read("tests/fixtures/test_binary_elf").expect("ELF fixture should exist");
+ let fixture_data = read_fixture("test_binary_elf", "ELF fixture should exist");
@@
- let fixture_data =
- std::fs::read("tests/fixtures/test_binary_elf").expect("ELF fixture should exist");
+ let fixture_data = read_fixture("test_binary_elf", "ELF fixture should exist");As per coding guidelines, "tests/{integration_.rs,test_.rs}: Store binary test fixtures in tests/fixtures/ and reference them using env!(\"CARGO_MANIFEST_DIR\") pattern".
Also applies to: 26-27, 39-40, 52-53, 66-67, 80-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration_cli_stdin.rs` around lines 13 - 14, Replace relative
fixture reads in tests/integration_cli_stdin.rs with manifest-rooted paths using
the CARGO_MANIFEST_DIR env macro: locate each std::fs::read call that currently
uses "tests/fixtures/..." (e.g., the fixture_data binding and the repeated reads
at the other mentioned locations) and construct the path by joining
env!("CARGO_MANIFEST_DIR") with "tests/fixtures/<filename>" before calling
std::fs::read so the tests no longer depend on the current working directory.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…stic - Remove recovery hints from ParseError and MemoryMapError #[error] attributes: ParseError is used for both format-detection and UTF-16 decode errors, so "ensure the file is a valid binary" is wrong in decode contexts. MemoryMapError is never constructed anywhere. - Add --min-len to filter-no-match diagnostic (was missing from the list of adjustable filters) - Tighten flow8 filter-no-match test to assert on hint text Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
unclesp1d3r
changed the title
unclesp1d3r
changed the title
Match the pattern used by cli_macho_binary in integration_cli.rs: allow graceful failure on platforms where Mach-O parsing may not work. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
There was a problem hiding this comment.
Pull request overview
Updates user-facing documentation and CLI UX to better match actual behavior, while adding integration tests around stdin usage and help text to prevent regressions.
Changes:
- Polishes CLI UX (remove
-eshort flag, add EXIT CODES to--help, improve filter-no-match diagnostic, make encoding conversion idiomatic). - Fixes and restructures documentation (README rewrite, correct MSRV/flags/exit codes, rewrite performance guide).
- Simplifies Mergify configuration and adds tooling config for Tessl MCP.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/main.rs |
Removes -e, adds EXIT CODES to help output, uses From<CliEncoding> for EncodingFilter, updates tag help text comments. |
src/pipeline/mod.rs |
Expands the “filters matched nothing” diagnostic with actionable hints. |
tests/integration_flow8_diagnostics.rs |
Tightens stderr assertions to require the new hint text. |
tests/integration_cli.rs |
Adds coverage that --help includes an EXIT CODES section. |
tests/integration_cli_short_flags.rs |
Updates short-flag tests to reflect removal of -e and use --enc. |
tests/integration_cli_stdin.rs |
Adds stdin-focused CLI integration coverage (formats, output modes, filters). |
README.md |
Rewrites README structure, adds badges/sections, updates examples and feature list. |
docs/src/installation.md |
Corrects MSRV and installation guidance; removes fictional Docker section. |
docs/src/performance.md |
Rewrites performance guide to match real flags/behavior and benchmark tooling. |
docs/src/quickstart.md |
Expands semantic tags table to include missing tags. |
docs/src/cli.md |
Updates exit code documentation to match StringyError::exit_code(). |
GOTCHAS.md |
Updates documented CLI conventions (short flags) and help-text sync guidance. |
AGENTS.md |
Updates the CLI flags reference table (removes -e). |
.mergify.yml |
Simplifies queue rules and bot auto-approval/merge requirements. |
tessl.json |
Adds vendored Tessl dependency configuration for skills. |
.mcp.json |
Adds MCP server config for Tessl. |
.gemini/settings.json |
Adds Gemini MCP server config for Tessl. |
.cursor/mcp.json |
Adds Cursor MCP server config for Tessl. |
.codex/config.toml |
Adds Codex MCP server config for Tessl. |
.gitignore |
Ignores Tessl-generated files/directories. |
| .arg("--enc") | ||
| .arg("ascii") | ||
| .assert() | ||
| .success(); |
| ```bash | ||
| cargo run -- --help | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Usage | ||
|
|
||
| ```bash | ||
| # Basic analysis with ranked output | ||
| # Ranked output with semantic tags | ||
| stringy target_binary | ||
|
|
||
| # Filter by semantic tags |
| - **PE resources**: VERSIONINFO, STRINGTABLE, and MANIFEST extraction | ||
| - **Import/export analysis**: Symbol extraction from all supported formats | ||
| - **Ranking**: Section-aware scoring with band-mapped 0-100 normalization | ||
| - **Deduplication**: Canonical string grouping with configurable similarity threshold |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tests/integration_cli_stdin.rs (1)
13-14:⚠️ Potential issue | 🟠 MajorUse manifest-rooted fixture paths for deterministic test execution.
These reads are cwd-dependent and can fail in non-default runners. Use
env!("CARGO_MANIFEST_DIR")-rooted paths (preferably through a helper) for all fixture loads.Proposed fix
+use std::path::PathBuf; use assert_cmd::Command; use assert_cmd::cargo_bin_cmd; use predicates::prelude::*; fn stringy() -> Command { cargo_bin_cmd!("stringy") } + +fn fixture_path(name: &str) -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("tests") + .join("fixtures") + .join(name) +} + +fn read_fixture(name: &str, msg: &str) -> Vec<u8> { + std::fs::read(fixture_path(name)).expect(msg) +} @@ - let fixture_data = - std::fs::read("tests/fixtures/test_binary_pe.exe").expect("PE fixture should exist"); + let fixture_data = read_fixture("test_binary_pe.exe", "PE fixture should exist"); @@ - let fixture_data = - std::fs::read("tests/fixtures/test_binary_macho").expect("Mach-O fixture should exist"); + let fixture_data = read_fixture("test_binary_macho", "Mach-O fixture should exist"); @@ - let fixture_data = - std::fs::read("tests/fixtures/test_unknown.bin").expect("unknown fixture should exist"); + let fixture_data = read_fixture("test_unknown.bin", "unknown fixture should exist"); @@ - let fixture_data = - std::fs::read("tests/fixtures/test_binary_elf").expect("ELF fixture should exist"); + let fixture_data = read_fixture("test_binary_elf", "ELF fixture should exist"); @@ - let fixture_data = - std::fs::read("tests/fixtures/test_binary_elf").expect("ELF fixture should exist"); + let fixture_data = read_fixture("test_binary_elf", "ELF fixture should exist"); @@ - let fixture_data = - std::fs::read("tests/fixtures/test_binary_elf").expect("ELF fixture should exist"); + let fixture_data = read_fixture("test_binary_elf", "ELF fixture should exist");As per coding guidelines, "tests/{integration_.rs,test_.rs}: Store binary test fixtures in
tests/fixtures/and reference them usingenv!(\"CARGO_MANIFEST_DIR\")pattern".Also applies to: 26-27, 42-43, 55-56, 69-70, 83-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration_cli_stdin.rs` around lines 13 - 14, The test uses cwd-dependent paths when reading fixtures (e.g., the let fixture_data = std::fs::read("tests/fixtures/test_binary_pe.exe") call); change all such reads in tests/integration_cli_stdin.rs (including the other occurrences referenced) to use manifest-rooted paths via env!("CARGO_MANIFEST_DIR") (or a small helper like fixture_path) so the file lookup is deterministic across runners; update the code that constructs fixture_data and the other fixture reads to join env!("CARGO_MANIFEST_DIR") with "tests/fixtures/..." instead of using relative paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integration_cli_stdin.rs`:
- Line 12: Rename the test functions to follow the project's test naming
convention test_<function>_<scenario>: for example change fn
stdin_pipe_pe_binary() to fn test_stdin_pipe_pe_binary_success() (or another
clear scenario suffix), and apply the same pattern to the other test functions
in this file so each name begins with "test_" and includes the function and
scenario (e.g., test_<originalName>_<scenario>).
- Around line 11-94: The tests in this file lack insta snapshot assertions;
update the three representative tests stdin_pipe_json_output,
stdin_pipe_raw_mode, and stdin_pipe_with_filters to capture the command stdout
via .output() (instead of .assert()), convert to a UTF-8 string (e.g., from
result.stdout using String::from_utf8_lossy), and replace the current
non-empty/success assertions with insta::assert_snapshot!(stdout_string) calls
(ensure the tests import the insta macro and produce stable snapshots for
JSON/raw/filtered outputs); keep existing success checks only if you still call
.status and include the stdout snapshot assertion immediately after running the
command.
---
Duplicate comments:
In `@tests/integration_cli_stdin.rs`:
- Around line 13-14: The test uses cwd-dependent paths when reading fixtures
(e.g., the let fixture_data = std::fs::read("tests/fixtures/test_binary_pe.exe")
call); change all such reads in tests/integration_cli_stdin.rs (including the
other occurrences referenced) to use manifest-rooted paths via
env!("CARGO_MANIFEST_DIR") (or a small helper like fixture_path) so the file
lookup is deterministic across runners; update the code that constructs
fixture_data and the other fixture reads to join env!("CARGO_MANIFEST_DIR") with
"tests/fixtures/..." instead of using relative paths.
🪄 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: 2ce6f332-fce4-422c-b315-4c73815416f3
📒 Files selected for processing (1)
tests/integration_cli_stdin.rs
| #[test] | ||
| fn stdin_pipe_pe_binary() { | ||
| let fixture_data = | ||
| std::fs::read("tests/fixtures/test_binary_pe.exe").expect("PE fixture should exist"); | ||
|
|
||
| stringy() | ||
| .arg("-") | ||
| .write_stdin(fixture_data) | ||
| .assert() | ||
| .success() | ||
| .stdout(predicate::str::is_empty().not()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn stdin_pipe_macho_binary() { | ||
| let fixture_data = | ||
| std::fs::read("tests/fixtures/test_binary_macho").expect("Mach-O fixture should exist"); | ||
|
|
||
| // Mach-O fixtures may not parse on all platforms; verify it runs without panicking | ||
| let result = stringy() | ||
| .arg("-") | ||
| .write_stdin(fixture_data) | ||
| .output() | ||
| .expect("should execute"); | ||
|
|
||
| assert!(result.status.success() || !result.stderr.is_empty()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn stdin_pipe_unknown_data() { | ||
| let fixture_data = | ||
| std::fs::read("tests/fixtures/test_unknown.bin").expect("unknown fixture should exist"); | ||
|
|
||
| // Unknown format falls back to unstructured byte scan (succeeds, may emit info) | ||
| stringy() | ||
| .arg("-") | ||
| .write_stdin(fixture_data) | ||
| .assert() | ||
| .success(); | ||
| } | ||
|
|
||
| #[test] | ||
| fn stdin_pipe_json_output() { | ||
| let fixture_data = | ||
| std::fs::read("tests/fixtures/test_binary_elf").expect("ELF fixture should exist"); | ||
|
|
||
| stringy() | ||
| .arg("-") | ||
| .arg("--json") | ||
| .write_stdin(fixture_data) | ||
| .assert() | ||
| .success() | ||
| .stdout(predicate::str::is_empty().not()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn stdin_pipe_raw_mode() { | ||
| let fixture_data = | ||
| std::fs::read("tests/fixtures/test_binary_elf").expect("ELF fixture should exist"); | ||
|
|
||
| stringy() | ||
| .arg("-") | ||
| .arg("--raw") | ||
| .write_stdin(fixture_data) | ||
| .assert() | ||
| .success() | ||
| .stdout(predicate::str::is_empty().not()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn stdin_pipe_with_filters() { | ||
| let fixture_data = | ||
| std::fs::read("tests/fixtures/test_binary_elf").expect("ELF fixture should exist"); | ||
|
|
||
| stringy() | ||
| .arg("-") | ||
| .arg("--min-len") | ||
| .arg("8") | ||
| .arg("--top") | ||
| .arg("10") | ||
| .write_stdin(fixture_data) | ||
| .assert() | ||
| .success(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify snapshot coverage in this integration test file.
rg -n --type=rust -C2 'assert_debug_snapshot!\(' tests/integration_cli_stdin.rs
rg -n --type=rust -C2 'stdout\(predicate::str::is_empty\(\)\.not\(\)\)|\.success\(\);' tests/integration_cli_stdin.rsRepository: EvilBit-Labs/Stringy
Length of output: 571
🏁 Script executed:
# Check if insta is a dependency
grep -E 'insta|test' Cargo.toml | head -20Repository: EvilBit-Labs/Stringy
Length of output: 123
🏁 Script executed:
# Find other integration test files and check for snapshot usage
find tests -name "integration_*.rs" -o -name "test_*.rs" | head -10Repository: EvilBit-Labs/Stringy
Length of output: 386
🏁 Script executed:
# Check for existing snapshot test patterns in integration tests
rg -l 'assert_debug_snapshot|assert_snapshot' tests/Repository: EvilBit-Labs/Stringy
Length of output: 338
🏁 Script executed:
# Check a sample of integration test files for snapshot usage
rg -n 'assert.*snapshot' tests/integration_cli.rs | head -10Repository: EvilBit-Labs/Stringy
Length of output: 46
🏁 Script executed:
# Check the specific file for any insta usage
rg -n 'insta|snapshot' tests/integration_cli_stdin.rsRepository: EvilBit-Labs/Stringy
Length of output: 46
🏁 Script executed:
# Check another integration test for comparison
rg -n 'assert.*snapshot' tests/integration_elf.rs | head -5Repository: EvilBit-Labs/Stringy
Length of output: 156
🏁 Script executed:
# Review the structure of one integration test using snapshots
head -50 tests/output_json_integration.rsRepository: EvilBit-Labs/Stringy
Length of output: 1402
🏁 Script executed:
# Check how assert_snapshot is used in the JSON integration test more fully
sed -n '30,45p' tests/output_json_integration.rsRepository: EvilBit-Labs/Stringy
Length of output: 455
🏁 Script executed:
# Verify the approach used in another integration test file
rg -A10 'assert_snapshot!' tests/integration_elf.rs | head -20Repository: EvilBit-Labs/Stringy
Length of output: 413
🏁 Script executed:
# Check how snapshot tests capture output in CLI integration tests
rg -B5 -A5 'get_output\|assert.*snapshot' tests/integration_cli.rs | head -30Repository: EvilBit-Labs/Stringy
Length of output: 46
🏁 Script executed:
# Check integration_cli.rs for snapshot patterns
rg -B3 -A8 'assert_snapshot' tests/integration_cli.rs | head -30Repository: EvilBit-Labs/Stringy
Length of output: 46
🏁 Script executed:
# Count how many tests exist in integration_cli_stdin.rs
rg -c '#\[test\]' tests/integration_cli_stdin.rsRepository: EvilBit-Labs/Stringy
Length of output: 65
🏁 Script executed:
# Look at the full context of integration_cli_stdin.rs to understand test structure
wc -l tests/integration_cli_stdin.rsRepository: EvilBit-Labs/Stringy
Length of output: 97
🏁 Script executed:
# Check if assert_debug_snapshot is used anywhere in the project
rg 'assert_debug_snapshot' tests/Repository: EvilBit-Labs/Stringy
Length of output: 207
🏁 Script executed:
# Verify the actual snapshot output patterns used in the project
rg -B2 'assert_snapshot!\(' tests/output_json_integration.rs tests/integration_elf.rs | head -25Repository: EvilBit-Labs/Stringy
Length of output: 1564
🏁 Script executed:
# Check if the review comment's .get_output() pattern is valid for assert_cmd
rg 'get_output\|write_stdin' tests/integration_cli_stdin.rsRepository: EvilBit-Labs/Stringy
Length of output: 46
🏁 Script executed:
# Check how other CLI tests capture and handle output
cat tests/integration_cli.rs | head -100Repository: EvilBit-Labs/Stringy
Length of output: 2496
🏁 Script executed:
# Verify the assert_cmd API used in tests
rg -B5 -A10 'write_stdin.*output\(' tests/ | head -30Repository: EvilBit-Labs/Stringy
Length of output: 46
🏁 Script executed:
# Check how snapshots are stored/named in the project
ls -la tests/snapshots/ | head -20Repository: EvilBit-Labs/Stringy
Length of output: 1839
🏁 Script executed:
# Verify the exact snapshot naming convention used
rg -B2 'assert_snapshot!\(' tests/output_json_integration.rs | grep 'fn test' | head -5Repository: EvilBit-Labs/Stringy
Length of output: 94
🏁 Script executed:
# Check the exact pattern for capturing output in integration_cli.rs
sed -n '20,30p' tests/integration_cli.rsRepository: EvilBit-Labs/Stringy
Length of output: 417
🏁 Script executed:
# Verify if .get_output() is the correct API
rg 'get_output\(\)' tests/Repository: EvilBit-Labs/Stringy
Length of output: 2911
🏁 Script executed:
# Check how assert_cmd is imported and used
head -5 tests/integration_cli.rsRepository: EvilBit-Labs/Stringy
Length of output: 190
🏁 Script executed:
# Check the snapshot naming patterns in integration_flows_1_5.rs for CLI tests
rg -B3 -A3 'assert_snapshot!' tests/integration_flows_1_5.rs | head -30Repository: EvilBit-Labs/Stringy
Length of output: 462
🏁 Script executed:
# Check if assert_debug_snapshot is used for CLI output or just for debugging data
rg -B5 -A2 'assert_debug_snapshot' tests/classification_integration.rsRepository: EvilBit-Labs/Stringy
Length of output: 369
Add snapshot testing to CLI stdin tests per guideline requirement.
Coding guidelines mandate insta snapshot testing for integration_*.rs files. Current tests only assert success/non-empty output, missing output regressions in formatting and content. Add snapshots for representative modes: --json, --raw, and filtered output.
The example should use assert_snapshot!() (not assert_debug_snapshot!()) for string CLI output:
Example direction
+use insta::assert_snapshot;
@@
#[test]
fn stdin_pipe_json_output() {
@@
let fixture_data =
std::fs::read("tests/fixtures/test_binary_elf").expect("ELF fixture should exist");
- stringy()
+ let assert = stringy()
.arg("-")
.arg("--json")
.write_stdin(fixture_data)
- .assert()
- .success()
- .stdout(predicate::str::is_empty().not());
+ .assert()
+ .success();
+
+ let stdout = String::from_utf8(assert.get_output().stdout.clone()).unwrap();
+ insta::assert_snapshot!("stdin_pipe_json_output", stdout);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration_cli_stdin.rs` around lines 11 - 94, The tests in this file
lack insta snapshot assertions; update the three representative tests
stdin_pipe_json_output, stdin_pipe_raw_mode, and stdin_pipe_with_filters to
capture the command stdout via .output() (instead of .assert()), convert to a
UTF-8 string (e.g., from result.stdout using String::from_utf8_lossy), and
replace the current non-empty/success assertions with
insta::assert_snapshot!(stdout_string) calls (ensure the tests import the insta
macro and produce stable snapshots for JSON/raw/filtered outputs); keep existing
success checks only if you still call .status and include the stdout snapshot
assertion immediately after running the command.
| // ---------- Stdin edge cases ---------- | ||
|
|
||
| #[test] | ||
| fn stdin_pipe_pe_binary() { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Align test names with the project test naming convention.
Rename these functions to test_<function>_<scenario> format for consistency and discoverability (for example, test_stdin_pipe_pe_binary_success).
As per coding guidelines, "tests/**/*.rs: Test names follow test_<function>_<scenario> pattern".
Also applies to: 25-25, 40-40, 53-53, 67-67, 81-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration_cli_stdin.rs` at line 12, Rename the test functions to
follow the project's test naming convention test_<function>_<scenario>: for
example change fn stdin_pipe_pe_binary() to fn
test_stdin_pipe_pe_binary_success() (or another clear scenario suffix), and
apply the same pattern to the other test functions in this file so each name
begins with "test_" and includes the function and scenario (e.g.,
test_<originalName>_<scenario>).
Summary
Overhaul user-facing documentation and CLI polish. Rewrites README with badges and user-focused structure, fixes stale docs (wrong MSRV, fictional CLI flags, incorrect exit codes), improves CLI UX from review findings, and simplifies Mergify merge queue config.
20 files changed (452 additions, 633 deletions -- net reduction of 181 lines)
What changed
README rewrite
flat-square, modeled after libmagic-rs/dbsurveyorDocumentation fixes
docs/src/installation.md: Fixed MSRV from 1.70 to 1.91, removed fictional Docker section, added Releases page link, addedjustdev workflowdocs/src/performance.md: Complete rewrite -- removed 20+ fictional CLI flags (--mmap-threshold,--no-mmap,--sections,--timing,--no-classify, etc.), fictional env vars (STRINGY_MAX_MEMORY,STRINGY_THREADS), and fictional config formatdocs/src/cli.md: Fixed exit codes from 0/1/2 to 0/1/2/3/4 matchingStringyError::exit_code()docs/src/quickstart.md: Added 4 missing tags to semantic tags table (demangled, version, manifest, resource) -- now covers all 21 tagsCLI improvements (from
/cli-review)-eshort flag from--enc(infrequent flag per GOTCHAS.md convention)--helpoutput (0=success, 1=general, 2=config, 3=not found, 4=permission denied)cli_encoding_to_filter()toimpl From<CliEncoding> for EncodingFilter(idiomatic Rust)--min-len,--only-tags,--no-tags, or--encMergify simplification
/queuefor human PRs)Tests (7 new tests)
tests/integration_cli_stdin.rs(6 tests): PE, Mach-O, unknown format, JSON output, raw mode, and filtered mode via stdintests/integration_cli.rs: exit codes help text validationtests/integration_flow8_diagnostics.rs: tightened filter-no-match assertion to verify hint textTooling config
.gitignorefor tessl-generated filesBreaking changes
-eshort flag removed: Scripts usingstringy -e asciimust change tostringy --enc ascii. The long form--encis unchanged.Test plan
cargo nextest run-- 617 passed, 0 failed, 23 skippedcargo clippy --all-targets --all-features -- -D warnings-- cleancargo fmt --all -- --check-- cleanjust ci-check-- all stages pass (pre-commit, fmt, clippy, tests, coverage, dist plan)stringy --helpshows EXIT CODES section--encno longer accepts-eshort form