fix(find): accept native find flags (-name, -type, etc.)#211
fix(find): accept native find flags (-name, -type, etc.)#211sahilmgandhi wants to merge 1 commit intortk-ai:masterfrom
Conversation
991b50b to
480d278
Compare
pszymkowiak
left a comment
There was a problem hiding this comment.
Nice work! Clean approach, consistent with how we handle git.rs and grep_cmd.rs (trailing_var_arg + manual parsing). Auto-detection of native vs RTK syntax is smart.
15 new tests, security is fine (-exec properly rejected), -iname and -maxdepth are good additions.
One thing to fix before merge:
In parse_native_find_args(), unknown flags are silently swallowed by the _ => {} catch-all. This means rtk find . -mindepth 2 -name "*.rs" succeeds but ignores -mindepth — the user thinks it works but gets wrong results.
Please add a warning on stderr for unknown flags:
_ => {
eprintln!("rtk find: unknown flag '{}', ignored", args[i]);
}This way users know their flag was dropped instead of getting silently wrong results.
Everything else looks good — small fix and we can merge 👍
|
Thanks for this PR! Clean approach, consistent with how we handle git.rs and grep_cmd.rs (trailing_var_arg + manual parsing). Auto-detection of native vs RTK syntax is smart. Tests are solid (15 new), security is fine (-exec properly rejected), and -iname / -maxdepth are nice additions. One thing to fix before merge: in parse_native_find_args(), unknown flags are silently swallowed by the _ => {} catch-all. I tested with -mindepth and it succeeds without warning — the _ => { Small fix and we can merge — thanks again! |
480d278 to
7a4b64c
Compare
|
Thank you for the review @pszymkowiak, I took a pass at it, and updated the PR accordingly. |
|
Thanks! It close #170 as well :) |
Review: PR #211The find implementation is solid — auto-detection of native vs RTK syntax, clear rejection of unsupported flags ( Local testing confirmed everything works:
One issue: the branch needs a rebase on current master ( Fix: |
Summary
rtk find . -name "*.rs" -type ffails withunexpected argument '-n' foundbecause Clap decomposes-nameinto short flags (-n,-a,-m,-e) and-nis not definedFindto usetrailing_var_arg = true, allow_hyphen_values = true(consistent withGrep,Ls,Tree, and most other RTK commands)parse_find_args()that detects and handles both native find syntax (-name,-type,-maxdepth,-iname) and existing RTK syntax (rtk find *.rs src -m 50 -t f)-not,-exec,-delete, etc.) with a clear error instead of silently giving wrong resultsChanges
src/main.rsFindargs withtrailing_var_arg+ dispatch torun_from_args()src/find_cmd.rsparse_find_args()with native/RTK syntax detection,FindArgswithDefault,next_arghelper. Fixed-maxdepthto wire toWalkBuilder::max_depth(traversal depth, not display limit).-inamenow performs case-insensitive glob matching. Unsupported flags (-not,-exec, etc.) bail with clear error. 15 new tests.Test plan
cargo fmt --allpassescargo clippy --all-targetscleancargo test find_cmd— 27/27 tests pass (15 new)cargo test --all— 409/409 tests passrtk find . -name "*.rs" -type f— works, returns compact output