uutils / findutils

Rust implementation of findutils
MIT License
309 stars 38 forks source link

Add support for embedded "{}" #213

Closed int3 closed 1 year ago

int3 commented 1 year ago

Given

find . -exec foo{}bar \;

GNU find will replace {} by the filename. This commit matches that behavior.

Note that this is not required by the POSIX spec.

The GNU testsuite doesn't cover this case, so I've added a test in this repo.

tavianator commented 1 year ago

It still didn't make a difference for reporting new passing tests, since it's still comparing against the last main build which was the previous PR

int3 commented 1 year ago

Ah I see. Is that a blocker for merging this?

int3 commented 1 year ago

Ping?

sylvestre commented 1 year ago

sorry, i would like to see more tests:

int3 commented 1 year ago

find_cmd_tests.rs please add similar tests here

You mean find_exec_tests.rs, right? Since this is an exec change

with some utf8 input

Are there any tests that cover utf8 that I could reference?

with some failures (for example: if the exec doesn't exist)

This seems kind of orthogonal to the PR -- regardless of whether there are multiple {} in a filename doesn't change whether the exec exists

int3 commented 1 year ago

Okay I found a bunch of utf8 tests in the coreutils repo but none in the findutils repo. I'm not against adding those tests but again they seem orthogonal to this PR

sylvestre commented 1 year ago

ok, don't hesitate if you want to add them :)