Repository navigation
Conversation
|
|
||
| impl IgnorePattern { | ||
| fn new(pattern: &OsStr) -> Result<Self, glob::PatternError> { | ||
| // Lossy decoding is sufficient for validating wildcard syntax. Matching |
There was a problem hiding this comment.
pattern is only used for validation when non_utf8, do we need both bytes and non_utf8? bytes: Option could carry it
|
please make the matching consistent (same matcher for UTF-8 and non-UTF-8 names), then it should be good to go. |
| let Some(pattern) = &self.bytes else { | ||
| return false; | ||
| }; | ||
| let Ok(name) = std::ffi::CString::new(name.as_encoded_bytes()) else { |
There was a problem hiding this comment.
this allocates a CString per entry and per pattern, could we avoid it?
| case_sensitive: true, | ||
| }; | ||
| #[cfg(all(unix, not(target_os = "redox")))] | ||
| if self.non_utf8 || name.to_str().is_none() { |
There was a problem hiding this comment.
same pattern now gives different results depending on the file name encoding: fnmatch handles a\* or [[:digit:]], the glob crate doesn't.
why not always use fnmatch on unix? it would also remove the dual path.
| let ts = TestScenario::new(util_name!()); | ||
| for name in [b"bad\xff.txt".as_slice(), "bad�.txt".as_bytes(), b"keep"] { | ||
| ts.fixtures.touch(OsStr::from_bytes(name)); | ||
| } |
There was a problem hiding this comment.
please also add a case with a UTF-8 pattern using \ or [[:alpha:]] against a non-UTF-8 name
| .args(&["--zero", option]) | ||
| .arg(OsStr::from_bytes(b"bad\xff.*")) | ||
| .succeeds(); | ||
| result.stdout_contains("bad�.txt"); |
There was a problem hiding this comment.
could be a simple stdout_only_bytes(b"bad\xef\xbf\xbd.txt\0keep\0"), no?
| .args(&["--zero", option, "bad*.txt"]) | ||
| .succeeds() | ||
| .stdout_only("keep\0"); | ||
| } |
There was a problem hiding this comment.
the comment says --ignore still applies but it isn't tested, please add it
de5f68e to
5c7fa73
Compare
| impl IgnorePattern { | ||
| fn new(pattern: &OsStr) -> Result<Self, glob::PatternError> { | ||
| #[cfg(all(unix, not(target_os = "redox")))] | ||
| parse_glob::from_str(&pattern.to_string_lossy())?; |
There was a problem hiding this comment.
we match with fnmatch but still check the pattern with the glob crate, so --ignore='a**b' or --ignore='[' gets a warning and is dropped. could we skip this check on unix?
| pattern: parse_glob::from_str(&pattern.to_string_lossy())?, | ||
| #[cfg(all(unix, not(target_os = "redox")))] | ||
| bytes: std::ffi::CString::new(pattern.as_encoded_bytes()).ok(), | ||
| }) |
There was a problem hiding this comment.
an argument can't contain a NUL, do we need the Option here? the .ok() would silently drop the pattern
5c7fa73 to
bc04ab3
Compare
Use native glob matching for non-UTF-8 Unix filenames and ignore/hide patterns. Add regressions distinguishing invalid bytes from literal replacement characters. Closes uutils#7161.
bc04ab3 to
2612968
Compare
Use native glob matching for non-UTF-8 Unix filenames and ignore/hide patterns. Add regressions distinguishing invalid bytes from literal replacement characters.
Closes #7161.