-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
find: Implement -anewer
and -cnewer
.
#386
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #386 +/- ##
==========================================
+ Coverage 58.70% 59.04% +0.33%
==========================================
Files 30 30
Lines 3821 3855 +34
Branches 841 850 +9
==========================================
+ Hits 2243 2276 +33
- Misses 1254 1255 +1
Partials 324 324 ☔ View full report in Codecov by Sentry. |
tests/find_cmd_tests.rs
Outdated
#[cfg(target_os = "linux")] | ||
let x_options = ["a", "c", "m"]; | ||
#[cfg(not(target_os = "linux"))] | ||
let x_options = ["a", "B", "c", "m"]; | ||
#[cfg(target_os = "linux")] | ||
let y_options = ["a", "c", "m"]; | ||
#[cfg(not(target_os = "linux"))] | ||
let y_options = ["a", "B", "c", "m"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As x_options
and y_options
contain the same values, I think it is unnecessary to have two vars. And so my suggestion is to simply use options
.
tests/find_cmd_tests.rs
Outdated
|
||
for &x in &x_options { | ||
for &y in &y_options { | ||
let arg = &format!("-newer{x}{y}").to_string(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling to_string()
is unnecessary because format!
already returns a string.
let arg = &format!("-newer{x}{y}").to_string(); | |
let arg = &format!("-newer{x}{y}"); |
tests/find_cmd_tests.rs
Outdated
for &x in &x_options { | ||
for &y in &y_options { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the references are not necessary and you can simply use for x in x_options
.
tests/find_cmd_tests.rs
Outdated
for &arg in &args { | ||
for &time in × { | ||
let arg = &format!("{arg}{time}").to_string(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my previous comments.
Thanks. I'm sorry for these issues, they have been fixed in new commits. |
Thanks. There's no need to be sorry, the things I mentioned are minor details ;-) And it's great that you fixed them in |
implement: #370
Added extra judgment to
parse_str_to_newer_args()
function.-anewer
is equivalent to-neweram
-cnewer
is equivalent to-newercm
-newer
is equicalent to-newermm
BFS test
+1 PASS
in commit 57f2a3b.This PR does not solve Parsing multiple time formats, I will open a new PR to handle it separately. : )