glob-tilde-expansion #149 #173
Conversation
Co-authored-by: Rene Leonhardt <[email protected]>
Co-authored-by: Rene Leonhardt <[email protected]>
Co-authored-by: Rene Leonhardt <[email protected]>
src/lib.rs
Outdated
| /// Enables or disables tilde (`~`) expansion in glob patterns | ||
| pub fn glob_tilde_expansion(mut self, v: bool) -> Self { | ||
| self.glob_tilde_expansion = v; | ||
| self | ||
| } |
There was a problem hiding this comment.
We don't really have a builder pattern here, so this isn't needed
| _ => {} | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
What is the intent of this branch? AFAIK ~foo without the path shouldn't be a valid pattern
There was a problem hiding this comment.
~followed by user name then~and username are substituted by the home directory of that user.- If the username is invalid, or the home directory cannot be determined, then no substitution is performed.
- if we want
~followed by not username or not/or the home directory cannot be determined as an error. we have to add a another field toMatchOptions. if the fiend is settruethe it will be return error. if the field setfalse, it ignore it.
(could i add an extra field to the MatcherOptiond ?)
There was a problem hiding this comment.
Missed that in the glob manpage, thanks.
I think we should defer that part, and error if there is ~ not followed by /. Checking the env isn't always accurate: it needs getpwuid_r on Unix and GetUserProfileDirectoryA on Windows, which is more complexity than this crate should add.
There was a problem hiding this comment.
alright, i will update it.
Co-authored-by: Rene Leonhardt <[email protected]>
Co-authored-by: Rene Leonhardt <[email protected]>
Co-authored-by: Rene Leonhardt <[email protected]>
src/utils.rs
Outdated
| #[cfg(not(target_os = "windows"))] | ||
| return std::env::var("USER").ok(); | ||
| #[cfg(target_os = "windows")] | ||
| std::env::var("USERNAME").ok() |
There was a problem hiding this comment.
This will go away with #173 (comment) but for future reference, you should use cfg! rather than #[cfg(...)] where possible (i.e. when the types are the same).
let varname = if cfg!(windows) { "USERNAME" } else { "USER" };
env::var(varname).ok()Makes no difference here but in general it avoids conditional compilation, so both branches get checked on all platforms. Also lets you use else.
There was a problem hiding this comment.
Good point, and even the merciless rust formatter doesn't bloat 1 line to 5 lines in this case at least 😍
env::var(if cfg!(windows) { "USERNAME" } else { "USER" }).ok()…ermine home dir or user name
MatchOptionsstruct, the test is faild. could i update or add new test or any changes let me know.