refactor: Use PathBuf for paths in flag parsing and whitelists #3955
Conversation
There was a problem hiding this comment.
Patch seems reasonable except for the glob part where I comment below...
What is the goal of this patch? does this fix something?
@bartlomieju PTAL
| check: bool, | ||
| ) -> Result<(), ErrBox> { | ||
| // TODO: improve glob to look for tsx?/jsx? files only | ||
| let glob_paths = maybe_files.unwrap_or_else(|| vec!["**/*".to_string()]); | ||
| let glob_paths = maybe_files.unwrap_or_else(|| vec![PathBuf::from("**/*")]); |
There was a problem hiding this comment.
This seems odd/wrong PathBuf::from("**/*") ...
There was a problem hiding this comment.
I agree with @ry - it'd be better to rename field on Subcommand::Format to globs and keep Vec<String> type.
Otherwise +1 from me, stricter types are always good
There was a problem hiding this comment.
As far as I've seen it's normal for glob patterns to be stored as "paths". They have distinctly separated segments (since any special syntax is limited to a segment), absoluteness, parent paths, it's useful to join() them, normalize() them, etc. Are you sure this is a problem?
There was a problem hiding this comment.
Yeah, makes sense, I've seen glob/globset crate do that. LGTM
denoland#3955)" This reverts commit 701ce9b.
Follows from #3714.