[WIP] change --filter to only affect fixing and display

JamieMason · Oct 28, 2024 · c47d958 · c47d958
1 parent 459b35e
commit c47d958
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 56 deletions.
diff --git a/src/cli.rs b/src/cli.rs
@@ -37,12 +37,17 @@ impl Cli {
 }
 
 fn filter_option(command: &str) -> Arg {
+  let operation = if command == "lint" { "display" } else { "fix" };
   Arg::new("filter")
     .long("filter")
     .long_help(cformat!(
-      r#"Only include dependencies whose <bold>name</bold> matches this <bold>RegEx</bold>
+      r#"Only <bold>{operation}</bold> dependencies whose <bold>name</bold> matches this <bold>RegEx</bold>
 
-<bold><underline>Examples</underline></bold>
+<bold><underline>Important:</underline></bold>
+--filter only affects what syncpack will <bold>{operation}</bold>. syncpack will still
+inspect and exit 1/0 based on every dependency in your project.
+
+<bold><underline>Examples:</underline></bold>
 <dim>An exact match for "react"</>
 <dim>$</dim> <yellow><bold>syncpack {command}</bold> --filter '^react$'</>
 <dim>Any name containing "react" anywhere within it</>
@@ -58,7 +63,7 @@ fn log_levels_option(command: &str) -> Arg {
     .long_help(cformat!(
       r#"Control how detailed the log output should be
 
-<bold><underline>Examples</underline></bold>
+<bold><underline>Examples:</underline></bold>
 <dim>Turn off logging completely</dim>
 <dim>$</dim> <yellow><bold>syncpack {command}</bold> --log-levels off</>
 <dim>Only show verbose debugging logs</dim>
@@ -77,7 +82,7 @@ fn no_ansi_option(command: &str) -> Arg {
     .long_help(cformat!(
       r#"Disable ANSI colored output and terminal hyperlinks
 
-<bold><underline>Examples</underline></bold>
+<bold><underline>Examples:</underline></bold>
 <dim>$</dim> <yellow><bold>syncpack {command}</bold> --no-ansi</>"#
     ))
     .action(clap::ArgAction::SetTrue)
@@ -89,7 +94,7 @@ fn only_option(command: &str) -> Arg {
     .long_help(cformat!(
       r#"Only inspect version mismatches, or formatting issues
 
-<bold><underline>Examples</underline></bold>
+<bold><underline>Examples:</underline></bold>
 <dim>Only inspect version mismatches</dim>
 <dim>$</dim> <yellow><bold>syncpack {command}</bold> --only mismatches</>
 <dim>Only inspect formatting of package.json files</dim>
@@ -106,14 +111,14 @@ fn show_option(command: &str) -> Arg {
     .long_help(cformat!(
       r#"Control what information is displayed in lint output
 
-<bold><underline>Values</underline></bold>
+<bold><underline>Values:</underline></bold>
 <yellow>ignored</>       Show instances and dependencies which syncpack is ignoring
 <yellow>instances</>     Show every instance of every dependency
 <yellow>local-hints</>   Show a hint alongside dependencies developed in this repo
 <yellow>packages</>      Show formatting status of each package.json file
 <yellow>status-codes</>  Show specifically how/why a dependency or instance is valid or invalid
 
-<bold><underline>Examples</underline></bold>
+<bold><underline>Examples:</underline></bold>
 <dim>Show highest level of detail</dim>
 <dim>$</dim> <yellow><bold>syncpack {command}</bold> --show ignored,instances,local-hints,packages,status-codes</>"#
     ))
@@ -128,10 +133,10 @@ fn source_option(command: &str) -> Arg {
     .long_help(cformat!(
       r#"A list of quoted glob patterns for package.json files to read from
 
-<bold><underline>Examples</underline></bold>
+<bold><underline>Examples:</underline></bold>
 <dim>$</dim> <yellow><bold>syncpack {command}</bold> --source 'package.json' --source 'apps/*/package.json'</>
 
-<bold><underline>Resolving Packages</underline></bold>
+<bold><underline>Resolving Packages:</underline></bold>
 Patterns are discovered in the following order, first one wins:
 
 1. <yellow>--source</> CLI options

diff --git a/src/effects/fix.rs b/src/effects/fix.rs
@@ -16,6 +16,8 @@ pub fn run(ctx: Context) -> Context {
     // @TODO: show_valid: false,
     // @TODO: sort_by: "name" | "state" | "count",
   };
+  let dependency_name_regex = ctx.config.cli.options.dependency_name_regex.as_ref();
+  let matches_filter = |name: &str| -> bool { dependency_name_regex.map_or(true, |regex| regex.is_match(name)) };
 
   if ctx.config.cli.options.inspect_mismatches {
     ui.print_command_header("SEMVER RANGES AND VERSION MISMATCHES");
@@ -26,6 +28,11 @@ pub fn run(ctx: Context) -> Context {
 
     ctx.instances.iter().for_each(|instance| {
       let name = &instance.name;
+
+      if !matches_filter(name) {
+        return;
+      }
+
       let location = ui.instance_location(instance).dimmed();
       let state = instance.state.borrow().clone();
       let state_name = state.get_name();

diff --git a/src/effects/lint.rs b/src/effects/lint.rs
@@ -11,6 +11,8 @@ pub fn run(ctx: Context) -> Context {
     // @TODO: show_valid: false,
     // @TODO: sort_by: "name" | "state" | "count",
   };
+  let dependency_name_regex = ctx.config.cli.options.dependency_name_regex.as_ref();
+  let matches_filter = |name: &str| -> bool { dependency_name_regex.map_or(true, |regex| regex.is_match(name)) };
 
   if ctx.config.cli.options.inspect_mismatches {
     ui.print_command_header("SEMVER RANGES AND VERSION MISMATCHES");
@@ -26,12 +28,14 @@ pub fn run(ctx: Context) -> Context {
         return;
       }
       group.dependencies.borrow().values().for_each(|dependency| {
-        ui.print_dependency(dependency, &group.variant);
-        ui.for_each_instance(dependency, |instance| {
-          if ctx.config.cli.options.show_instances {
-            ui.print_instance(instance, &group.variant);
-          }
-        });
+        if matches_filter(&dependency.name) {
+          ui.print_dependency(dependency, &group.variant);
+          ui.for_each_instance(dependency, |instance| {
+            if ctx.config.cli.options.show_instances {
+              ui.print_instance(instance, &group.variant);
+            }
+          });
+        }
       });
     });
   }

diff --git a/src/packages.rs b/src/packages.rs
@@ -78,15 +78,6 @@ impl Packages {
   where
     F: FnMut(Instance),
   {
-    let matches_filter = |name: &str| -> bool {
-      config
-        .cli
-        .options
-        .dependency_name_regex
-        .as_ref()
-        .map_or(true, |regex| regex.is_match(name))
-    };
-
     for package in self.all.iter() {
       for dependency_type in &config.rcfile.get_all_dependency_types() {
         match dependency_type.strategy {
@@ -103,54 +94,46 @@ impl Packages {
                 }
               }),
             ) {
-              if matches_filter(&name) {
-                on_instance(Instance::new(
-                  name.to_string(),
-                  raw_specifier.to_string(),
-                  dependency_type,
-                  Rc::clone(package),
-                ));
-              }
+              on_instance(Instance::new(
+                name.to_string(),
+                raw_specifier.to_string(),
+                dependency_type,
+                Rc::clone(package),
+              ));
             }
           }
           Strategy::NamedVersionString => {
             if let Some(Value::String(specifier)) = package.borrow().get_prop(&dependency_type.path) {
               if let Some((name, raw_specifier)) = specifier.split_once('@') {
-                if matches_filter(name) {
-                  on_instance(Instance::new(
-                    name.to_string(),
-                    raw_specifier.to_string(),
-                    dependency_type,
-                    Rc::clone(package),
-                  ));
-                }
-              }
-            }
-          }
-          Strategy::UnnamedVersionString => {
-            if let Some(Value::String(raw_specifier)) = package.borrow().get_prop(&dependency_type.path) {
-              if matches_filter(&dependency_type.name) {
                 on_instance(Instance::new(
-                  dependency_type.name.clone(),
+                  name.to_string(),
                   raw_specifier.to_string(),
                   dependency_type,
                   Rc::clone(package),
                 ));
               }
             }
           }
+          Strategy::UnnamedVersionString => {
+            if let Some(Value::String(raw_specifier)) = package.borrow().get_prop(&dependency_type.path) {
+              on_instance(Instance::new(
+                dependency_type.name.clone(),
+                raw_specifier.to_string(),
+                dependency_type,
+                Rc::clone(package),
+              ));
+            }
+          }
           Strategy::VersionsByName => {
             if let Some(Value::Object(versions_by_name)) = package.borrow().get_prop(&dependency_type.path) {
               for (name, raw_specifier) in versions_by_name {
-                if matches_filter(&name) {
-                  if let Value::String(version) = raw_specifier {
-                    on_instance(Instance::new(
-                      name.to_string(),
-                      version.to_string(),
-                      dependency_type,
-                      Rc::clone(package),
-                    ));
-                  }
+                if let Value::String(version) = raw_specifier {
+                  on_instance(Instance::new(
+                    name.to_string(),
+                    version.to_string(),
+                    dependency_type,
+                    Rc::clone(package),
+                  ));
                 }
               }
             }