Skip to content

Commit

Permalink
feat: change CLI file patterns to be intersection with the config and…
Browse files Browse the repository at this point in the history
… excludes to be a union (#804)
  • Loading branch information
dsherret committed Dec 18, 2023
1 parent 11ba56b commit c06fc0b
Show file tree
Hide file tree
Showing 12 changed files with 357 additions and 125 deletions.
40 changes: 31 additions & 9 deletions crates/dprint/src/arg_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,10 @@ pub enum HiddenSubCommand {

#[derive(Debug, Default, PartialEq, Eq)]
pub struct FilePatternArgs {
pub file_patterns: Vec<String>,
pub exclude_file_patterns: Vec<String>,
pub include_patterns: Vec<String>,
pub include_pattern_overrides: Option<Vec<String>>,
pub exclude_patterns: Vec<String>,
pub exclude_pattern_overrides: Option<Vec<String>>,
pub allow_node_modules: bool,
}

Expand Down Expand Up @@ -267,22 +269,24 @@ fn inner_parse_args<TStdInReader: StdInReader>(args: Vec<String>, std_in_reader:
LogLevel::Info
},
config: matches.get_one::<String>("config").map(String::from),
plugins: values_to_vec(matches.get_many("plugins")),
plugins: maybe_values_to_vec(matches.get_many("plugins")),
})
}

fn parse_file_patterns(matches: &ArgMatches) -> Result<FilePatternArgs> {
let plugins = values_to_vec(matches.get_many("plugins"));
let file_patterns = values_to_vec(matches.get_many("files"));
let plugins = maybe_values_to_vec(matches.get_many("plugins"));
let file_patterns = maybe_values_to_vec(matches.get_many("files"));

if !plugins.is_empty() && file_patterns.is_empty() {
validate_plugin_args_when_no_files(&plugins)?;
}

Ok(FilePatternArgs {
allow_node_modules: matches.get_flag("allow-node-modules"),
file_patterns,
exclude_file_patterns: values_to_vec(matches.get_many("excludes")),
include_patterns: file_patterns,
include_pattern_overrides: matches.get_many("includes-override").map(values_to_vec),
exclude_patterns: maybe_values_to_vec(matches.get_many("excludes")),
exclude_pattern_overrides: matches.get_many("excludes-override").map(values_to_vec),
})
}

Expand All @@ -296,8 +300,12 @@ fn parse_incremental(matches: &ArgMatches) -> Option<bool> {
}
}

fn values_to_vec(values: Option<clap::parser::ValuesRef<String>>) -> Vec<String> {
values.map(|x| x.map(std::string::ToString::to_string).collect()).unwrap_or_default()
fn maybe_values_to_vec(values: Option<clap::parser::ValuesRef<String>>) -> Vec<String> {
values.map(values_to_vec).unwrap_or_default()
}

fn values_to_vec(values: clap::parser::ValuesRef<String>) -> Vec<String> {
values.map(std::string::ToString::to_string).collect()
}

/// Users have accidentally specified: dprint fmt --plugins <url1> <url2> -- <file-path>
Expand Down Expand Up @@ -580,13 +588,27 @@ impl ClapExtensions for clap::Command {
self
.arg(
Arg::new("files")
.help("List of file patterns in quotes to format. This can be a subset of what is found in the config file.")
.num_args(1..),
)
.arg(
Arg::new("includes-override")
.long("includes-override")
.value_name("patterns")
.help("List of file patterns in quotes to format. This overrides what is specified in the config file.")
.num_args(1..),
)
.arg(
Arg::new("excludes")
.long("excludes")
.value_name("patterns")
.help("List of file patterns or directories in quotes to exclude when formatting. This excludes in addition to what is found in the config file.")
.num_args(1..),
)
.arg(
Arg::new("excludes-override")
.long("excludes-override")
.value_name("patterns")
.help("List of file patterns or directories in quotes to exclude when formatting. This overrides what is specified in the config file.")
.num_args(1..),
)
Expand Down
6 changes: 4 additions & 2 deletions crates/dprint/src/commands/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,10 @@ pub async fn update_plugins_config_file<TEnvironment: Environment>(
}

let file_pattern_args = FilePatternArgs {
file_patterns: Vec::new(),
exclude_file_patterns: Vec::new(),
include_patterns: Vec::new(),
include_pattern_overrides: None,
exclude_patterns: Vec::new(),
exclude_pattern_overrides: None,
allow_node_modules: false,
};
let scopes = resolve_plugins_scope_and_paths(args, &file_pattern_args, environment, plugin_resolver).await?;
Expand Down
105 changes: 90 additions & 15 deletions crates/dprint/src/commands/formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1098,7 +1098,7 @@ mod test {
#[cfg(target_os = "windows")]
#[test]
fn should_format_absolute_paths_on_windows() {
let file_path = "D:\\test\\other\\file1.txt"; // needs to be in the base directory
let file_path = "D:\\test\\other\\asdf\\file1.txt"; // needs to be in the base directory
let environment = TestEnvironmentBuilder::with_remote_wasm_plugin()
.with_local_config("D:\\test\\other\\dprint.json", |c| {
c.add_includes("asdf/**/*.txt").add_remote_wasm_plugin();
Expand All @@ -1115,10 +1115,10 @@ mod test {
assert_eq!(environment.read_file(&file_path).unwrap(), "text1_formatted");
}

#[cfg(target_os = "linux")]
#[cfg(unix)]
#[test]
fn should_format_absolute_paths_on_linux() {
let file_path = "/test/other/file1.txt"; // needs to be in the base directory
fn should_format_absolute_paths_on_unix() {
let file_path = "/test/other/asdf/file1.txt"; // needs to be in the base directory
let environment = TestEnvironmentBuilder::with_remote_wasm_plugin()
.with_local_config("/test/other/dprint.json", |c| {
c.add_includes("asdf/**/*.txt").add_remote_wasm_plugin();
Expand All @@ -1128,7 +1128,7 @@ mod test {
.initialize()
.build();

run_test_cli(vec!["fmt", "--", "/test/other/file1.txt"], &environment).unwrap();
run_test_cli(vec!["fmt", "--", "/test/other/asdf/file1.txt"], &environment).unwrap();

assert_eq!(environment.take_stdout_messages(), vec![get_singular_formatted_text()]);
assert_eq!(environment.read_file(&file_path).unwrap(), "text1_formatted");
Expand Down Expand Up @@ -1223,12 +1223,14 @@ mod test {
}

#[test]
fn should_override_config_excludes_with_cli_excludes() {
fn should_combine_config_excludes_with_cli_excludes() {
let file_path1 = "/file1.txt";
let file_path2 = "/file2.txt";
let file_path3 = "/file3.txt";
let environment = TestEnvironmentBuilder::with_remote_wasm_plugin()
.write_file(&file_path1, "text1")
.write_file(&file_path2, "text2")
.write_file(&file_path3, "text3")
.with_default_config(|c| {
c.add_excludes("/file1.txt").add_remote_wasm_plugin();
})
Expand All @@ -1237,13 +1239,34 @@ mod test {

run_test_cli(vec!["fmt", "--excludes", "/file2.txt"], &environment).unwrap();

assert_eq!(environment.take_stdout_messages(), vec![get_singular_formatted_text()]);
assert_eq!(environment.read_file(&file_path1).unwrap(), "text1");
assert_eq!(environment.read_file(&file_path2).unwrap(), "text2");
assert_eq!(environment.read_file(&file_path3).unwrap(), "text3_formatted");
}

#[test]
fn should_override_config_excludes_with_cli_excludes_override() {
let file_path1 = "/file1.txt";
let file_path2 = "/file2.txt";
let environment = TestEnvironmentBuilder::with_remote_wasm_plugin()
.write_file(&file_path1, "text1")
.write_file(&file_path2, "text2")
.with_default_config(|c| {
c.add_excludes("/file1.txt").add_remote_wasm_plugin();
})
.initialize()
.build();

run_test_cli(vec!["fmt", "--excludes-override", "/file2.txt"], &environment).unwrap();

assert_eq!(environment.take_stdout_messages(), vec![get_singular_formatted_text()]);
assert_eq!(environment.read_file(&file_path1).unwrap(), "text1_formatted");
assert_eq!(environment.read_file(&file_path2).unwrap(), "text2");
}

#[test]
fn should_support_clearing_config_excludes_with_cli_excludes_arg() {
fn should_support_clearing_config_excludes_with_cli_excludes_override_arg() {
let file_path1 = "/file1.txt";
let environment = TestEnvironmentBuilder::with_remote_wasm_plugin()
.write_file(&file_path1, "text1")
Expand All @@ -1253,7 +1276,7 @@ mod test {
.initialize()
.build();

run_test_cli(vec!["fmt", "--excludes="], &environment).unwrap();
run_test_cli(vec!["fmt", "--excludes-override="], &environment).unwrap();

assert_eq!(environment.take_stdout_messages(), vec![get_singular_formatted_text()]);
assert_eq!(environment.read_file(&file_path1).unwrap(), "text1_formatted");
Expand All @@ -1276,7 +1299,51 @@ mod test {
}

#[test]
fn should_override_config_includes_and_excludes_with_cli() {
fn should_combine_config_excludes_with_cli_args() {
let file_path1 = "/file1.txt";
let file_path2 = "/sub/file2.txt";
let file_path3 = "/sub/file3.txt";
let file_path4 = "/sub/file4.txt";
let environment = TestEnvironmentBuilder::with_remote_wasm_plugin()
.write_file(&file_path1, "text1")
.write_file(&file_path2, "text2")
.write_file(&file_path3, "text3")
.write_file(&file_path4, "text4")
.with_default_config(|c| {
c.add_includes("/sub/**/*.txt").add_excludes("/sub/file4.txt").add_remote_wasm_plugin();
})
.initialize()
.build();
run_test_cli(vec!["fmt", "--excludes", "/sub/file3.txt"], &environment).unwrap();

assert_eq!(environment.take_stdout_messages(), vec![get_singular_formatted_text()]);
assert_eq!(environment.read_file(&file_path1).unwrap(), "text1");
assert_eq!(environment.read_file(&file_path2).unwrap(), "text2_formatted");
assert_eq!(environment.read_file(&file_path3).unwrap(), "text3");
assert_eq!(environment.read_file(&file_path4).unwrap(), "text4");
}

#[test]
fn should_format_intersect_of_config_includes_and_cli_includes() {
let file_path1 = "/sub/file1.txt";
let file_path2 = "/sub/file2.txt";
let environment = TestEnvironmentBuilder::with_remote_wasm_plugin()
.write_file(&file_path1, "text1")
.write_file(&file_path2, "text2")
.with_default_config(|c| {
c.add_includes("/sub/**/*.txt").add_remote_wasm_plugin();
})
.initialize()
.build();
run_test_cli(vec!["fmt", "/sub/file2.txt"], &environment).unwrap();

assert_eq!(environment.take_stdout_messages(), vec![get_singular_formatted_text()]);
assert_eq!(environment.read_file(&file_path1).unwrap(), "text1");
assert_eq!(environment.read_file(&file_path2).unwrap(), "text2_formatted");
}

#[test]
fn should_override_config_includes_and_excludes_with_cli_overrides() {
let file_path1 = "/file1.txt";
let file_path2 = "/file2.txt";
let environment = TestEnvironmentBuilder::with_remote_wasm_plugin()
Expand All @@ -1287,7 +1354,11 @@ mod test {
})
.initialize()
.build();
run_test_cli(vec!["fmt", "/file1.txt", "--excludes", "/file2.txt"], &environment).unwrap();
run_test_cli(
vec!["fmt", "--includes-override", "/file1.txt", "--excludes-override", "/file2.txt"],
&environment,
)
.unwrap();

assert_eq!(environment.take_stdout_messages(), vec![get_singular_formatted_text()]);
assert_eq!(environment.read_file(&file_path1).unwrap(), "text1_formatted");
Expand Down Expand Up @@ -1785,7 +1856,6 @@ mod test {

#[test]
fn should_format_for_stdin_with_absolute_paths() {
// it should not output anything when downloading plugins
let environment = TestEnvironmentBuilder::with_initialized_remote_wasm_plugin()
.with_default_config(|c| {
c.add_includes("/src/**.*").add_remote_wasm_plugin();
Expand All @@ -1798,12 +1868,17 @@ mod test {
run_test_cli_with_stdin(vec!["fmt", "--stdin", "/file.txt"], &environment, test_std_in.clone()).unwrap();
assert_eq!(environment.take_stdout_messages(), vec!["text"]);

// make it matching on the cli
run_test_cli_with_stdin(vec!["fmt", "--stdin", "/file.txt", "--", "**/*.txt"], &environment, test_std_in.clone()).unwrap();
// matching file
run_test_cli_with_stdin(vec!["fmt", "--stdin", "/src/file.txt"], &environment, test_std_in.clone()).unwrap();
assert_eq!(environment.take_stdout_messages(), vec!["text_formatted"]);

// matching file
run_test_cli_with_stdin(vec!["fmt", "--stdin", "/src/file.txt"], &environment, test_std_in).unwrap();
// override what's in the config when overriding
run_test_cli_with_stdin(
vec!["fmt", "--stdin", "/file.txt", "--includes-override", "**/*.txt"],
&environment,
test_std_in,
)
.unwrap();
assert_eq!(environment.take_stdout_messages(), vec!["text_formatted"]);
}

Expand Down
28 changes: 25 additions & 3 deletions crates/dprint/src/commands/general.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,6 @@ mod test {
.add_remote_wasm_plugin()
.with_default_config(|config_file| {
config_file
.add_includes("**/*.txt")
.add_config_section(
"test-plugin",
r#"{
Expand Down Expand Up @@ -335,20 +334,30 @@ mod test {
fn providing_includes_to_cli_should_not_override_negated_includes() {
let environment = TestEnvironmentBuilder::with_initialized_remote_wasm_plugin()
.with_default_config(|c| {
c.add_includes("**/*.txt").add_includes("!sub/file4.txt");
c.add_includes("**/*.txt")
.add_includes("!sub/file4.txt")
// opt out
.add_includes("!sub3/sub/**/*.txt")
// then opt in
.add_includes("sub3/sub/dir/file.txt");
})
.write_file("/file.txt", "const t=4;")
.write_file("/file2.txt", "const t=4;")
.write_file("/sub/file3.txt", "const t=4;")
.write_file("/sub/file4.txt", "const t=4;")
.write_file("/sub2/file5.txt", "const t=4;")
.write_file("/sub3/sub/dir/file.txt", "const t=4;")
.write_file("/sub3/sub/dir/ignored.txt", "const t=4;")
.build();
// make sure it works as expected with no args
{
run_test_cli(vec!["output-file-paths"], &environment).unwrap();
let mut logged_messages = environment.take_stdout_messages();
logged_messages.sort();
assert_eq!(logged_messages, vec!["/file.txt", "/file2.txt", "/sub/file3.txt", "/sub2/file5.txt",]);
assert_eq!(
logged_messages,
vec!["/file.txt", "/file2.txt", "/sub/file3.txt", "/sub2/file5.txt", "/sub3/sub/dir/file.txt"]
);
}
// now provide an includes
{
Expand All @@ -363,6 +372,19 @@ mod test {
]
);
}
// try another one
{
run_test_cli(vec!["output-file-paths", "./sub3/**/*.*"], &environment).unwrap();
let mut logged_messages = environment.take_stdout_messages();
logged_messages.sort();
assert_eq!(
logged_messages,
vec![
// should not have the ingored.txt here
"/sub3/sub/dir/file.txt",
]
);
}
}

#[test]
Expand Down
27 changes: 2 additions & 25 deletions crates/dprint/src/configuration/resolve_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use crate::environment::CanonicalizedPathBuf;
use crate::environment::Environment;
use crate::plugins::parse_plugin_source_reference;
use crate::plugins::PluginSourceReference;
use crate::utils::is_negated_glob;
use crate::utils::resolve_url_or_file_path;
use crate::utils::PathSource;
use crate::utils::PluginKind;
Expand Down Expand Up @@ -125,12 +124,8 @@ pub async fn resolve_config_from_path<TEnvironment: Environment>(
}
// =========

let mut includes = take_array_from_config_map(&mut config_map, "includes")?;
let mut excludes = take_array_from_config_map(&mut config_map, "excludes")?;

// Move the negated includes to the excludes so that when someone provides
// includes on the command line, then it will still ignore these
move_negated_includes_to_excludes(&mut includes, &mut excludes);
let includes = take_array_from_config_map(&mut config_map, "includes")?;
let excludes = take_array_from_config_map(&mut config_map, "excludes")?;

let incremental = take_bool_from_config_map(&mut config_map, "incremental")?;
config_map.remove("projectType"); // this was an old config property that's no longer used
Expand All @@ -149,24 +144,6 @@ pub async fn resolve_config_from_path<TEnvironment: Environment>(
Ok(resolve_extends(resolved_config, extends, base_source, environment.clone()).await?)
}

fn move_negated_includes_to_excludes(includes: &mut Option<Vec<String>>, excludes: &mut Option<Vec<String>>) {
let Some(includes) = includes else {
return;
};
for i in (0..includes.len()).rev() {
if is_negated_glob(&includes[i]) {
let value = includes.remove(i);
if excludes.is_none() {
*excludes = Some(Vec::new());
}
// Make negated includes to have a higher priority than excludes.
// This means when checking if something is not matched, it will
// look at these first.
excludes.as_mut().unwrap().insert(0, value);
}
}
}

fn resolve_extends<TEnvironment: Environment>(
mut resolved_config: ResolvedConfig,
extends: Vec<String>,
Expand Down
Loading

0 comments on commit c06fc0b

Please sign in to comment.