Skip to content
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

fix: make include fields options #8

Merged
merged 1 commit into from
Sep 7, 2023

Conversation

nayeemrmn
Copy link
Collaborator

@nayeemrmn nayeemrmn commented Sep 7, 2023

Will fix .. denoland/deno#20393.

CLI patch:
diff --git a/cli/args/mod.rs b/cli/args/mod.rs
index f82ed9a71..e6ada9453 100644
--- a/cli/args/mod.rs
+++ b/cli/args/mod.rs
@@ -1398,20 +1398,18 @@ fn resolve_files(
   let mut result = maybe_files_config.unwrap_or_default();
   if let Some(file_flags) = maybe_file_flags {
     if !file_flags.include.is_empty() {
-      result.include = file_flags.include;
+      result.include = Some(file_flags.include);
     }
     if !file_flags.ignore.is_empty() {
       result.exclude = file_flags.ignore;
     }
   }
   // Now expand globs if there are any
-  if !result.include.is_empty() {
-    result.include = expand_globs(result.include)?;
-  }
-
-  if !result.exclude.is_empty() {
-    result.exclude = expand_globs(result.exclude)?;
-  }
+  result.include = match result.include {
+    Some(include) => Some(expand_globs(include)?),
+    None => None,
+  };
+  result.exclude = expand_globs(result.exclude)?;
 
   Ok(result)
 }
@@ -1624,7 +1622,7 @@ mod test {
     let temp_dir_path = temp_dir.path().as_path();
     let error = resolve_files(
       Some(FilesConfig {
-        include: vec![temp_dir_path.join("data/**********.ts")],
+        include: Some(vec![temp_dir_path.join("data/**********.ts")]),
         exclude: vec![],
       }),
       None,
@@ -1634,12 +1632,12 @@ mod test {
 
     let resolved_files = resolve_files(
       Some(FilesConfig {
-        include: vec![
+        include: Some(vec![
           temp_dir_path.join("data/test1.?s"),
           temp_dir_path.join("nested/foo/*.ts"),
           temp_dir_path.join("nested/fizz/*.ts"),
           temp_dir_path.join("pages/[id].ts"),
-        ],
+        ]),
         exclude: vec![temp_dir_path.join("nested/**/*bazz.ts")],
       }),
       None,
@@ -1648,7 +1646,7 @@ mod test {
 
     assert_eq!(
       resolved_files.include,
-      vec![
+      Some(vec![
         temp_dir_path.join("data/test1.js"),
         temp_dir_path.join("data/test1.ts"),
         temp_dir_path.join("nested/foo/bar.ts"),
@@ -1660,7 +1658,7 @@ mod test {
         temp_dir_path.join("nested/fizz/fizz.ts"),
         temp_dir_path.join("nested/fizz/foo.ts"),
         temp_dir_path.join("pages/[id].ts"),
-      ]
+      ])
     );
     assert_eq!(
       resolved_files.exclude,
diff --git a/cli/tools/bench/mod.rs b/cli/tools/bench/mod.rs
index d876c2504..732d889eb 100644
--- a/cli/tools/bench/mod.rs
+++ b/cli/tools/bench/mod.rs
@@ -429,7 +429,9 @@ pub async fn run_benchmarks_with_watch(
         let bench_options = cli_options.resolve_bench_options(bench_flags)?;
 
         let _ = sender.send(cli_options.watch_paths());
-        let _ = sender.send(bench_options.files.include.clone());
+        if let Some(include) = &bench_options.files.include {
+          let _ = sender.send(include.clone());
+        }
 
         let graph_kind = cli_options.type_check_mode().as_graph_kind();
         let module_graph_builder = factory.module_graph_builder().await?;
diff --git a/cli/tools/coverage/mod.rs b/cli/tools/coverage/mod.rs
index c77f869b2..6dca349f7 100644
--- a/cli/tools/coverage/mod.rs
+++ b/cli/tools/coverage/mod.rs
@@ -573,7 +573,11 @@ fn collect_coverages(
   .ignore_node_modules()
   .ignore_vendor_folder()
   .add_ignore_paths(&files.ignore)
-  .collect_files(&files.include)?;
+  .collect_files(if files.include.is_empty() {
+    None
+  } else {
+    Some(&files.include)
+  })?;
 
   for file_path in file_paths {
     let json = fs::read_to_string(file_path.as_path())?;
diff --git a/cli/tools/fmt.rs b/cli/tools/fmt.rs
index 5ab82795c..284f20dda 100644
--- a/cli/tools/fmt.rs
+++ b/cli/tools/fmt.rs
@@ -153,7 +153,7 @@ fn collect_fmt_files(files: &FilesConfig) -> Result<Vec<PathBuf>, AnyError> {
     .ignore_node_modules()
     .ignore_vendor_folder()
     .add_ignore_paths(&files.exclude)
-    .collect_files(&files.include)
+    .collect_files(files.include.as_deref())
 }
 
 /// Formats markdown (using <https://github.com/dprint/dprint-plugin-markdown>) and its code blocks
diff --git a/cli/tools/lint.rs b/cli/tools/lint.rs
index 753a9c08b..835b91f60 100644
--- a/cli/tools/lint.rs
+++ b/cli/tools/lint.rs
@@ -200,7 +200,7 @@ fn collect_lint_files(files: &FilesConfig) -> Result<Vec<PathBuf>, AnyError> {
     .ignore_node_modules()
     .ignore_vendor_folder()
     .add_ignore_paths(&files.exclude)
-    .collect_files(&files.include)
+    .collect_files(files.include.as_deref())
 }
 
 pub fn print_rules_list(json: bool, maybe_rules_tags: Option<Vec<String>>) {
diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs
index 15f8d2597..87ca7f3f5 100644
--- a/cli/tools/test/mod.rs
+++ b/cli/tools/test/mod.rs
@@ -1199,7 +1199,9 @@ pub async fn run_tests_with_watch(
         let test_options = cli_options.resolve_test_options(test_flags)?;
 
         let _ = sender.send(cli_options.watch_paths());
-        let _ = sender.send(test_options.files.include.clone());
+        if let Some(include) = &test_options.files.include {
+          let _ = sender.send(include.clone());
+        }
 
         let graph_kind = cli_options.type_check_mode().as_graph_kind();
         let log_level = cli_options.log_level();
diff --git a/cli/util/fs.rs b/cli/util/fs.rs
index 2ccd70b6d..93403659a 100644
--- a/cli/util/fs.rs
+++ b/cli/util/fs.rs
@@ -239,14 +239,13 @@ impl<TFilter: Fn(&Path) -> bool> FileCollector<TFilter> {
 
   pub fn collect_files(
     &self,
-    files: &[PathBuf],
+    files: Option<&[PathBuf]>,
   ) -> Result<Vec<PathBuf>, AnyError> {
     let mut target_files = Vec::new();
-    let files = if files.is_empty() {
-      // collect files in the current directory when empty
-      Cow::Owned(vec![PathBuf::from(".")])
-    } else {
+    let files = if let Some(files) = files {
       Cow::Borrowed(files)
+    } else {
+      Cow::Owned(vec![PathBuf::from(".")])
     };
     for file in files.iter() {
       if let Ok(file) = canonicalize_path(file) {
@@ -312,11 +311,10 @@ pub fn collect_specifiers(
     .ignore_vendor_folder();
 
   let root_path = current_dir()?;
-  let include_files = if files.include.is_empty() {
-    // collect files in the current directory when empty
-    Cow::Owned(vec![root_path.clone()])
+  let include_files = if let Some(include) = &files.include {
+    Cow::Borrowed(include)
   } else {
-    Cow::Borrowed(&files.include)
+    Cow::Owned(vec![root_path.clone()])
   };
   for path in include_files.iter() {
     let path = path.to_string_lossy();
@@ -336,7 +334,7 @@ pub fn collect_specifiers(
     };
     let p = normalize_path(p);
     if p.is_dir() {
-      let test_files = file_collector.collect_files(&[p])?;
+      let test_files = file_collector.collect_files(Some(&[p]))?;
       let mut test_files_as_urls = test_files
         .iter()
         .map(|f| ModuleSpecifier::from_file_path(f).unwrap())
@@ -776,7 +774,7 @@ mod tests {
     .add_ignore_paths(&[ignore_dir_path.to_path_buf()]);
 
     let result = file_collector
-      .collect_files(&[root_dir_path.to_path_buf()])
+      .collect_files(Some(&[root_dir_path.to_path_buf()]))
       .unwrap();
     let expected = [
       "README.md",
@@ -803,7 +801,7 @@ mod tests {
       .ignore_node_modules()
       .ignore_vendor_folder();
     let result = file_collector
-      .collect_files(&[root_dir_path.to_path_buf()])
+      .collect_files(Some(&[root_dir_path.to_path_buf()]))
       .unwrap();
     let expected = [
       "README.md",
@@ -823,10 +821,10 @@ mod tests {
 
     // test opting out of ignoring by specifying the dir
     let result = file_collector
-      .collect_files(&[
+      .collect_files(Some(&[
         root_dir_path.to_path_buf(),
         root_dir_path.to_path_buf().join("child/node_modules/"),
-      ])
+      ]))
       .unwrap();
     let expected = [
       "README.md",
@@ -894,11 +892,11 @@ mod tests {
 
     let result = collect_specifiers(
       &FilesConfig {
-        include: vec![
+        include: Some(vec![
           PathBuf::from("http://localhost:8080"),
           root_dir_path.to_path_buf(),
           PathBuf::from("https://localhost:8080".to_string()),
-        ],
+        ]),
         exclude: vec![ignore_dir_path.to_path_buf()],
       },
       predicate,
@@ -933,11 +931,11 @@ mod tests {
     };
     let result = collect_specifiers(
       &FilesConfig {
-        include: vec![PathBuf::from(format!(
+        include: Some(vec![PathBuf::from(format!(
           "{}{}",
           scheme,
           root_dir_path.join("child").to_string().replace('\\', "/")
-        ))],
+        ))]),
         exclude: vec![],
       },
       predicate,

cc @bartlomieju

@bartlomieju bartlomieju merged commit a8d29b3 into denoland:main Sep 7, 2023
3 checks passed
@nayeemrmn nayeemrmn deleted the include-as-option branch September 7, 2023 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants