Skip to content

Add support for embedded "{}"#213

Merged
sylvestre merged 1 commit into
uutils:mainfrom
int3:embedded
Mar 13, 2023
Merged

Add support for embedded "{}"#213
sylvestre merged 1 commit into
uutils:mainfrom
int3:embedded

Conversation

@int3

@int3 int3 commented Feb 26, 2023

Copy link
Copy Markdown
Contributor

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.

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

Copy link
Copy Markdown
Contributor

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

int3 commented Feb 28, 2023

Copy link
Copy Markdown
Contributor Author

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

@int3

int3 commented Mar 5, 2023

Copy link
Copy Markdown
Contributor Author

Ping?

@sylvestre

Copy link
Copy Markdown
Contributor

sorry, i would like to see more tests:

  • find_cmd_tests.rs please add similar tests here
  • with some utf8 input
  • with some failures (for example: if the exec doesn't exist)
    thanks

@int3

int3 commented Mar 11, 2023

Copy link
Copy Markdown
Contributor Author

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

int3 commented Mar 11, 2023

Copy link
Copy Markdown
Contributor Author

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 sylvestre merged commit 5659812 into uutils:main Mar 13, 2023
@sylvestre

Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants