Skip to content

Conversation

@milkory
Copy link
Contributor

@milkory milkory commented Mar 27, 2025

This PR implements -exec ... {} + with argmax as a new dependency.

Solve #6 , #366

@codecov
Copy link

codecov bot commented Mar 27, 2025

Codecov Report

Attention: Patch coverage is 66.10169% with 60 lines in your changes missing coverage. Please review.

Project coverage is 87.50%. Comparing base (1a40442) to head (b54004f).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/find/matchers/exec.rs 62.36% 27 Missing and 8 partials ⚠️
src/find/matchers/mod.rs 69.38% 6 Missing and 9 partials ⚠️
src/find/matchers/logical_matchers.rs 50.00% 8 Missing ⚠️
src/find/mod.rs 89.47% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #515      +/-   ##
==========================================
+ Coverage   87.47%   87.50%   +0.02%     
==========================================
  Files          31       31              
  Lines        6770     6914     +144     
  Branches      305      326      +21     
==========================================
+ Hits         5922     6050     +128     
- Misses        628      629       +1     
- Partials      220      235      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@milkory milkory requested a review from tavianator March 31, 2025 00:16

// Reset command status.
*command = self.new_command();
command.try_arg(&path_to_file).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's possible for this to fail for particularly long file paths. We should report an error rather than panic if that happens

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a check on that, thanks.

if let Err(e) = build_top_level_matcher(&["-exec", "+"], &mut config) {
assert!(e.to_string().contains("missing argument"));
} else {
panic!("parsing argument list with exec and no executable should fail");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use expect_err() for these?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are just copied from other tests. And use expect_err will cause dyn find::matchers::Matcher doesn't implement Debug. I don't know how to handle it.

Copy link
Collaborator

@hanbings hanbings left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, will there be any other updates here? : )

@milkory
Copy link
Contributor Author

milkory commented Apr 1, 2025

I don't have more updates if no further issue.

@hanbings hanbings merged commit 5235d78 into uutils:main Apr 2, 2025
19 of 20 checks passed
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.

Fix: Error: -exec [args...] + isn't supported yet. Only -exec [args...] ; FeatureComplete: implement exec[dir] +

3 participants