-
-
Notifications
You must be signed in to change notification settings - Fork 676
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
Replace String with PathBuf for paths #59
Conversation
Changes looks mostly good. Seems that it even fixes not excluding folders like Also probably this is better fix to #44 than 3b63798 The only thing for now which is broken is saving to file, because it prints records with one empty line between them
|
Windows excluded directories are broken Executing command
On Linux |
czkawka_core/src/big_file.rs
Outdated
for expression in &self.excluded_items.items { | ||
if Common::regex_check(expression, ¤t_file_name) { | ||
continue 'dir; | ||
} | ||
} | ||
|
||
#[cfg(target_family = "windows")] | ||
{ | ||
if cfg!(target_family = "windows") { | ||
current_file_name = Common::prettier_windows_path(¤t_file_name); |
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.
prettier_windows_path
function was used only for normalizing path like c:/Av
, c:\AV
, C:/av
to C:/av
to allow easily remove duplicate path or overlap ones.
With PathBuf I don't think that this is necessary, and can be removed(of course if everything else will work fine)
Pathbuf doesn't recognize that on Windows
and the only solution for me is to check every single time when using excluding directories if string path converted to lowerspace are equal |
Right, I'll fix this when I got some time. |
You can try to rebase to current master, because I added recently CI job with checking simple regressions and it probably will allow you to test PR without needing using Windows or Wine(The longest job takes 6/7 minutes). |
I have extracted some duplicated code related to exclusion into a separated methods. Please check whether it's OK. |
Seems that everything works now. |
This PR changes handling of paths so that
String
is replaced withPathBuf
. The latter is more suitable for this purpose as it is cross-platform and is not required to be valid UTF8. Also provides nice convenience methods likeextension
orjoin
that correctly handles separators on various platforms.