-
-
Notifications
You must be signed in to change notification settings - Fork 718
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
Implement threaded search in rest of modes(except empty folders) #504
Conversation
czkawka_core/src/duplicate.rs
Outdated
self.text_messages.warnings.extend(warnings); | ||
for (length, fe) in fe_result { | ||
self.files_with_identical_size.entry(length).or_insert_with(Vec::new); | ||
self.files_with_identical_size.get_mut(&length).unwrap().push(fe); |
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.
This may be, with additional changes in loop above:
for fe in fe_result {
self.files_with_identical_size.entry(fe.size).or_insert_with(Vec::new);
self.files_with_identical_size.get_mut(&fe.size).unwrap().push(fe);
self.finish().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.
why moving this to the end of file?
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.
Without big reasons,
Just I like to have the most important and basic things at the top of file like e.g. enums
czkawka_core/src/empty_files.rs
Outdated
let file_name_lowercase: String = match entry_data.file_name().into_string() { | ||
Ok(t) => t, | ||
Err(_inspected) => { | ||
println!("File {:?} has not valid UTF-8 name", entry_data); |
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.
why would this be a println!
and not a warning?
czkawka_core/src/same_music.rs
Outdated
if !allowed_extensions.iter().any(|r| current_file_name.to_string_lossy().ends_with(r)) { | ||
continue 'dir; | ||
} | ||
if ![".mp3", ".flac", ".m4a"].iter().any(|r| file_name_lowercase.ends_with(r)) { |
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.
could you implement an intersection of allowed_extensions
set with set [".mp3", ".flac", ".m4a"]
? Thus replacing two checks with a check against set intersection.
czkawka_core/src/similar_images.rs
Outdated
similarity: Similarity::None, | ||
}; | ||
let allowed_image_extensions = [".jpg", ".jpeg", ".png" /*, ".bmp"*/, ".tiff", ".tif", ".tga", ".ff" /*, ".gif"*/, ".jif", ".jfi" /*, ".webp"*/]; // webp cannot be seen in preview, gif needs to be enabled after releasing image crate 0.24.0, bmp needs to be fixed in image crate | ||
if !allowed_image_extensions.iter().any(|e| file_name_lowercase.ends_with(e)) { |
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.
same concern about set intersection as before
czkawka_core/src/similar_videos.rs
Outdated
continue 'dir; | ||
} | ||
|
||
if ![".mp4", ".mpv", ".flv", ".mp4a", ".webm", ".mpg", ".mp2", ".mpeg", ".m4p", ".m4v", ".avi", ".wmv", ".qt", ".mov", ".swf", ".mkv"] |
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.
same concern with set intersection
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.
also: such a big constant should be moved outside of the function into a const
czkawka_core/src/similar_videos.rs
Outdated
error: "".to_string(), | ||
}; | ||
|
||
fe_result.push((entry_data.file_name().to_string_lossy().to_string(), fe)); |
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.
was full path with current_folder.join(entry_data.file_name())
, now is entry_data.file_name()
, why the change and what does that mean?
Looks good to me. |
Probably something more generic should be used, but for now it is just OK