Sort ignore_dirs before to build GlobSet #223
Conversation
| assert_eq!(rel_path, PathBuf::from("main.cpp")); | ||
| assert_eq!(result, empty_result!()); | ||
| let mut ignore_dirs = vec!["mydir/*".to_string(), "mydir2/*".to_string()]; | ||
| for _ in 0..2 { |
There was a problem hiding this comment.
Nit: please add a comment here saying that you run the test twice, with different ordering of ignore_dirs.
| assert_eq!(rel_path, PathBuf::from("main.cpp")); | ||
| assert_eq!(result, empty_result!()); | ||
| let mut ignore_dirs = vec!["mydir\\*".to_string(), "mydir2\\*".to_string()]; | ||
| for _ in 0..2 { |
| filter_option: Option<bool>, | ||
| ) -> CovResultIter { | ||
| let mut glob_builder = GlobSetBuilder::new(); | ||
| to_ignore_dirs.sort_unstable(); |
There was a problem hiding this comment.
Nit: can you add a comment here, explaining why this is necessary (and pointing to the globset bug)
We should also file an issue so we remember to remove this when it's no longer necessary.
| @@ -175,10 +175,11 @@ pub fn rewrite_paths( | |||
| source_dir: Option<PathBuf>, | |||
| prefix_dir: Option<PathBuf>, | |||
| ignore_not_existing: bool, | |||
| to_ignore_dirs: Vec<String>, | |||
| mut to_ignore_dirs: Vec<String>, | |||
There was a problem hiding this comment.
It would be nice not to require mutation, but I guess the only way would be to clone the Vec (or use an iterator, but it would either require a complex implementation or using a third-party library such as https://docs.rs/itertools/0.5.8/itertools/trait.Itertools.html#method.sorted).
So, let's keep the mut.
There was a problem hiding this comment.
My first idea was to insert-sort the vec when building it in main.rs and so not to have this "mut" here.
This reverts commit 8d3a0d4.
No description provided.