tests/integration_tests.rs (2)

193-209: Edge cases for the filter(|s| !s.is_empty()) guard are untested

The implementation correctly strips empty segments from inputs like |src|docs| or src||docs, but none of these inputs are exercised. A few cheap extra assertions (or a separate test) would give confidence that the guard works end-to-end.

🧪 Suggested additions

// Leading/trailing pipes should still exclude both dirs
let output_pipe_edges = cmd()
    .args(["-I", "|src|docs|", temp_dir.path().to_str().unwrap()])
    .assert()
    .success()
    .get_output()
    .stdout
    .clone();
assert!(output_not_contains(&output_pipe_edges, "src"));
assert!(output_not_contains(&output_pipe_edges, "docs"));

// Consecutive pipes (empty segment between them) should be ignored
let output_double_pipe = cmd()
    .args(["-I", "src||docs", temp_dir.path().to_str().unwrap()])
    .assert()
    .success()
    .get_output()
    .stdout
    .clone();
assert!(output_not_contains(&output_double_pipe, "src"));
assert!(output_not_contains(&output_double_pipe, "docs"));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/integration_tests.rs` around lines 193 - 209, Add two assertions
exercising the filter(|s| !s.is_empty()) guard: invoke cmd() like the existing
tests but pass "-I" with a leading/trailing pipe string ("|src|docs|") and with
consecutive pipes ("src||docs"), capture stdout into new variables (e.g.,
output_pipe_edges and output_double_pipe) and assert that both "src" and "docs"
are excluded using output_not_contains, while other entries still exist; this
mirrors the existing output_pipe_exclude flow and verifies that empty segments
are ignored end-to-end.

---

204-205: output_not_contains uses an unrestricted substring match

output_not_contains(&output_pipe_exclude, "src") will spuriously fail if the temporary directory path printed as the tree root contains the substring "src". System TempDir paths rarely contain it, but anchoring the check to a tree-formatted line (e.g. prefixed by a connector or newline) makes the assertion more resilient and expressive.

🔒 Example of a more anchored assertion

-    assert!(output_not_contains(&output_pipe_exclude, "src"));
-    assert!(output_not_contains(&output_pipe_exclude, "docs"));
+    // Match the entry as it appears in tree output (preceded by a connector or newline)
+    let out_str = String::from_utf8_lossy(&output_pipe_exclude);
+    assert!(!out_str.contains("\nsrc\n") && !out_str.contains("── src"));
+    assert!(!out_str.contains("\ndocs\n") && !out_str.contains("── docs"));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/integration_tests.rs` around lines 204 - 205, The assertions use
output_not_contains(&output_pipe_exclude, "src") which can false-positive if the
temp dir path contains "src"; update the checks to anchor to tree-formatted
lines instead of raw substring matching: in the test that references
output_pipe_exclude replace the plain "src" and "docs" checks with assertions
that look for tree-line patterns (e.g. check the output does not contain "\n├──
src", "\n└── src" and similarly for "docs" or use a regex that matches
line-start plus tree connector and the folder name). Locate the calls to
output_not_contains and the variables output_pipe_exclude/output_pipe_include in
the failing test and change them to use the anchored tree-line patterns or
anchored regexes so only actual tree entries are matched.

src/rust_tree/cli.rs (2)

194-200: Add a unit test for the pipe-splitting path in cli_to_options

The existing test_cli_to_options_with_patterns only exercises a single non-pipe exclude string. The integration test covers end-to-end behaviour, but a focused unit test for the splitting logic in this module would catch regressions at a finer granularity.

🧪 Suggested unit test

#[test]
fn test_cli_to_options_pipe_separated_exclude() {
    let cli = Cli {
        path: ".".to_string(),
        exclude: vec!["src|docs".to_string()],
        // ... all other fields set to their defaults ...
        all_files: false, level: None, dir_only: false, no_indent: false,
        print_size: false, human_readable: false, pattern: None,
        full_path: false, color: false, no_color: false, ascii: false,
        sort_by_time: false, reverse: false, print_mod_date: false,
        output_file: None, file_limit: None, dirs_first: false,
        classify: false, no_report: false, print_permissions: false,
        fromfile: false, icons: false, prune: false, match_dirs: false,
    };

    let options = cli_to_options(&cli).unwrap();
    assert_eq!(options.exclude_patterns.len(), 2);
    assert!(options.exclude_patterns[0].matches("src"));
    assert!(options.exclude_patterns[1].matches("docs"));
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/rust_tree/cli.rs` around lines 194 - 200, Add a focused unit test that
constructs a Cli (using the Cli struct) with exclude containing a pipe-separated
string like "src|docs", calls cli_to_options(&cli) and asserts the returned
Options has two parsed patterns in exclude_patterns (using parse_glob_pattern
behavior): check length == 2 and that each Pattern matches the expected string
(e.g., options.exclude_patterns[0].matches("src") and [1].matches("docs")).
Place the test alongside existing tests for cli_to_options to exercise the
pipe-splitting path.

---

197-198: Whitespace around | is not trimmed — user-facing UX gap

Splitting on '|' without trimming means -I "src | docs" (spaces around the separator) produces glob patterns "src " and " docs", which won't match directory names src or docs. Consider adding a .map(|s| s.trim()) before the filter to absorb surrounding whitespace.

✂️ Proposed change

     .flat_map(|s| s.split('|'))
+    .map(|s| s.trim())
     .filter(|s| !s.is_empty())

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/rust_tree/cli.rs` around lines 197 - 198, The split/join iterator chain
currently does .flat_map(|s| s.split('|')).filter(|s| !s.is_empty()) which
leaves surrounding whitespace on each chunk (e.g., " src "); insert a .map(|s|
s.trim()) between the flat_map and filter so each segment is trimmed before
emptiness check (i.e., .flat_map(|s| s.split('|')).map(|s| s.trim()).filter(|s|
!s.is_empty())), ensuring user-provided patterns like `src | docs` match
correctly.

src/rust_tree/cli.rs (1)

360-396: Consider testing invalid-pattern error propagation through a pipe-separated string.

The happy-path split of "src|docs" is covered, but the collect::<Result<Vec<_>, _>>()? short-circuit path — triggered when one fragment is an invalid glob (e.g. "src|[") — has no unit-test coverage. This exercises a different code path from test_cli_to_options_invalid_pattern, which only tests a bare invalid pattern, not one embedded inside a pipe-separated value.

✨ Suggested additional test

#[test]
fn test_cli_to_options_pipe_separated_exclude_invalid() {
    let cli = Cli {
        path: ".".to_string(),
        exclude: vec!["src|[".to_string()], // '[' is an invalid glob fragment
        all_files: false,
        level: None,
        dir_only: false,
        no_indent: false,
        print_size: false,
        human_readable: false,
        pattern: None,
        full_path: false,
        color: false,
        no_color: false,
        ascii: false,
        sort_by_time: false,
        reverse: false,
        print_mod_date: false,
        output_file: None,
        file_limit: None,
        dirs_first: false,
        classify: false,
        no_report: false,
        print_permissions: false,
        fromfile: false,
        icons: false,
        prune: false,
        match_dirs: false,
    };
    assert!(cli_to_options(&cli).is_err());
}

As a secondary note: the test body duplicates the non-default field values from test_cli_to_options_with_patterns (e.g. all_files: true, level: Some(2), etc.), none of which affect exclude_patterns. Since Cli has no Default derive, the verbosity is structurally forced — but deriving Default for Cli (all booleans/Options/Vecs already have zero-values; path would need a manual override or a helper factory) would allow future tests to use Cli { exclude: vec![...], ..Cli::default() } and express intent more clearly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/rust_tree/cli.rs` around lines 360 - 396, The test suite lacks coverage
for the error path when a pipe-separated exclude string contains an invalid glob
fragment; add a new unit test (e.g.
test_cli_to_options_pipe_separated_exclude_invalid) that constructs a Cli with
exclude: vec!["src|[".to_string()] and asserts that
cli_to_options(&cli).is_err(), to ensure the collect::<Result<Vec<_>, _>>()?
short-circuits on invalid fragments; optionally consider deriving Default for
Cli (or add a small helper) so future tests can set only the differing fields
(exclude) using ..Cli::default() to reduce boilerplate.

Configuration used: defaults

Review profile: CHILL

Plan: Pro