diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e720122e9..299894fa70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Deprecations +* The original configuration syntax for `jj fix` is now deprecated in favor of + one that allows defining multiple tools that can affect different filesets. + These can be used in combination for now. See `jj help fix` for details. + ### New features * External diff tools can now be configured to invoke the tool on each file @@ -85,6 +89,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * [The default commit description template](docs/config.md#default-description) can now be configured by `templates.draft_commit_description`. +* `jj fix` can now be configured to run different tools on different filesets. + This simplifies the use case of configuring code formatters for specific file + types. See `jj help fix` for details. + ### Fixed bugs * `jj diff --git` no longer shows the contents of binary files. diff --git a/cli/src/commands/fix.rs b/cli/src/commands/fix.rs index 590dfb9948..43eb5c13a6 100644 --- a/cli/src/commands/fix.rs +++ b/cli/src/commands/fix.rs @@ -19,10 +19,12 @@ use std::sync::mpsc::channel; use futures::StreamExt; use itertools::Itertools; -use jj_lib::backend::{BackendError, BackendResult, CommitId, FileId, TreeValue}; +use jj_lib::backend::{BackendError, CommitId, FileId, TreeValue}; +use jj_lib::fileset::{self, FilesetExpression}; +use jj_lib::matchers::{EverythingMatcher, Matcher}; use jj_lib::merged_tree::MergedTreeBuilder; use jj_lib::repo::Repo; -use jj_lib::repo_path::RepoPathBuf; +use jj_lib::repo_path::{RepoPathBuf, RepoPathUiConverter}; use jj_lib::revset::{RevsetExpression, RevsetIteratorExt}; use jj_lib::store::Store; use pollster::FutureExt; @@ -31,8 +33,8 @@ use rayon::prelude::ParallelIterator; use tracing::instrument; use crate::cli_util::{CommandHelper, RevisionArg}; -use crate::command_error::{config_error_with_message, CommandError}; -use crate::config::CommandNameAndArgs; +use crate::command_error::{config_error, CommandError}; +use crate::config::{to_toml_value, CommandNameAndArgs}; use crate::ui::Ui; /// Update files with formatting fixes or other changes @@ -42,26 +44,60 @@ use crate::ui::Ui; /// It can also be used to modify files with other tools like `sed` or `sort`. /// /// The changed files in the given revisions will be updated with any fixes -/// determined by passing their file content through the external tool. -/// Descendants will also be updated by passing their versions of the same files -/// through the same external tool, which will never result in new conflicts. -/// Files with existing conflicts will be updated on all sides of the conflict, -/// which can potentially increase or decrease the number of conflict markers. +/// determined by passing their file content through any external tools the user +/// has configured for those files. Descendants will also be updated by passing +/// their versions of the same files through the same tools, which will ensure +/// that the fixes are not lost. This will never result in new conflicts. Files +/// with existing conflicts will be updated on all sides of the conflict, which +/// can potentially increase or decrease the number of conflict markers. /// -/// The external tool must accept the current file content on standard input, -/// and return the updated file content on standard output. The output will not -/// be used unless the tool exits with a successful exit code. Output on -/// standard error will be passed through to the terminal. +/// The external tools must accept the current file content on standard input, +/// and return the updated file content on standard output. A tool's output will +/// not be used unless it exits with a successful exit code. Output on standard +/// error will be passed through to the terminal. /// -/// The configuration schema is expected to change in the future. For now, it -/// defines a single command that will affect all changed files in the specified -/// revisions. For example, to format some Rust code changed in the working copy -/// revision, you could write this configuration: +/// Tools are defined in a table where the keys are arbitrary identifiers and +/// the values have the following properties: +/// - `command`: The arguments used to run the tool. The first argument is the +/// path to an executable file. Arguments can contain the substring `$path`, +/// which will be replaced with the repo-relative path of the file being +/// fixed. It is useful to provide the path to tools that include the path in +/// error messages, or behave differently based on the directory or file +/// name. +/// - `patterns`: Determines which files the tool will affect. If this list is +/// empty, no files will be affected by the tool. If there are multiple +/// patterns, the tool is applied only once to each file in the union of the +/// patterns. +/// +/// For example, the following configuration defines how two code formatters +/// (`clang-format` and `black`) will apply to three different file extensions +/// (.cc, .h, and .py): +/// +/// [fix.tools.clang-format] +/// command = ["/usr/bin/clang-format", "--assume-filename=$path"] +/// patterns = ["glob:'**/*.cc'", +/// "glob:'**/*.h'"] +/// +/// [fix.tools.black] +/// command = ["/usr/bin/black", "-", "--stdin-filename=$path"] +/// patterns = ["glob:'**/*.py'"] +/// +/// Execution order of tools that affect the same file is deterministic, but +/// currently unspecified, and may change between releases. If two tools affect +/// the same file, the second tool to run will receive its input from the +/// output of the first tool. +/// +/// There is also a deprecated configuration schema that defines a single +/// command that will affect all changed files in the specified revisions. For +/// example, the following configuration would apply the Rust formatter to all +/// changed files (whether they are Rust files or not): /// /// [fix] /// tool-command = ["rustfmt", "--emit", "stdout"] /// -/// And then run the command `jj fix -s @`. +/// The tool defined by `tool-command` acts as if it was the first entry in +/// `fix.tools`, and uses `pattern = "all()"``. Support for `tool-command` +/// will be removed in a future version. #[derive(clap::Args, Clone, Debug)] #[command(verbatim_doc_comment)] pub(crate) struct FixArgs { @@ -82,6 +118,7 @@ pub(crate) fn cmd_fix( args: &FixArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; + let tools_config = get_tools_config(ui, command.settings().config())?; let root_commits: Vec = if args.source.is_empty() { workspace_command.parse_revset(&RevisionArg::from( command.settings().config().get_string("revsets.fix")?, @@ -166,15 +203,9 @@ pub(crate) fn cmd_fix( } // Run the configured tool on all of the chosen inputs. - // TODO: Support configuration of multiple tools and which files they affect. - let tool_command: CommandNameAndArgs = command - .settings() - .config() - .get("fix.tool-command") - .map_err(|err| config_error_with_message("Invalid `fix.tool-command`", err))?; let fixed_file_ids = fix_file_ids( tx.repo().store().as_ref(), - &tool_command, + &tools_config, &unique_tool_inputs, )?; @@ -261,20 +292,38 @@ struct ToolInput { /// each failed input. fn fix_file_ids<'a>( store: &Store, - tool_command: &CommandNameAndArgs, + tools_config: &ToolsConfig, tool_inputs: &'a HashSet, -) -> BackendResult> { +) -> Result, CommandError> { let (updates_tx, updates_rx) = channel(); // TODO: Switch to futures, or document the decision not to. We don't need // threads unless the threads will be doing more than waiting for pipes. tool_inputs.into_par_iter().try_for_each_init( || updates_tx.clone(), - |updates_tx, tool_input| -> Result<(), BackendError> { - let mut read = store.read_file(&tool_input.repo_path, &tool_input.file_id)?; - let mut old_content = vec![]; - read.read_to_end(&mut old_content).unwrap(); - if let Ok(new_content) = run_tool(tool_command, tool_input, &old_content) { - if new_content != *old_content { + |updates_tx, tool_input| -> Result<(), CommandError> { + let mut matching_tools = tools_config + .tools + .iter() + .filter(|tool_config| tool_config.matcher.matches(&tool_input.repo_path)) + .peekable(); + if matching_tools.peek().is_some() { + // The first matching tool gets its input from the committed file, and any + // subsequent matching tool gets its input from the previous matching tool's + // output. + let mut old_content = vec![]; + let mut read = store.read_file(&tool_input.repo_path, &tool_input.file_id)?; + read.read_to_end(&mut old_content)?; + let new_content = + matching_tools.fold(old_content.clone(), |prev_content, tool_config| { + match run_tool(&tool_config.command, tool_input, &prev_content) { + Ok(next_content) => next_content, + // TODO: Because the stderr is passed through, this isn't always failing + // silently, but it should do something better will the exit code, tool + // name, etc. + Err(_) => prev_content, + } + }); + if new_content != old_content { let new_file_id = store.write_file(&tool_input.repo_path, &mut new_content.as_slice())?; updates_tx.send((tool_input, new_file_id)).unwrap(); @@ -328,3 +377,104 @@ fn run_tool( Err(()) } } + +/// Represents an entry in the `fix.tools` config table. +struct ToolConfig { + /// The command that will be run to fix a matching file. + command: CommandNameAndArgs, + /// The matcher that determines if this tool matches a file. + matcher: Box, + // TODO: Store the `name` field here and print it with the command's stderr, to clearly + // associate any errors/warnings with the tool and its configuration entry. +} + +/// Represents the `fix.tools` config table. +struct ToolsConfig { + /// Some tools, stored in the order they will be executed if more than one + /// of them matches the same file. + tools: Vec, +} + +/// Simplifies deserialization of the config values while building a ToolConfig. +#[derive(Clone, Debug, Eq, PartialEq, serde::Deserialize)] +#[serde(rename_all = "kebab-case")] +struct RawToolConfig { + command: CommandNameAndArgs, + patterns: Vec, +} + +/// Parses the `fix.tools` config table. +/// +/// Parses the deprecated `fix.tool-command` config as if it was the first entry +/// in `fix.tools`. +/// +/// Fails if any of the commands or patterns are obviously unusable, but does +/// not check for issues that might still occur later like missing executables. +/// This is a place where we could fail earlier in some cases, though. +fn get_tools_config(ui: &mut Ui, config: &config::Config) -> Result { + let mut tools_config = ToolsConfig { tools: Vec::new() }; + // TODO: Remove this block of code and associated documentation after at least + // one release where the feature is marked deprecated. + if let Ok(tool_command) = config.get::("fix.tool-command") { + // This doesn't change the displayed indices of the `fix.tools` definitions, and + // doesn't have a `name` that could conflict with them. That would matter more + // if we already had better error handling that made use of the `name`. + tools_config.tools.push(ToolConfig { + command: tool_command, + matcher: Box::new(EverythingMatcher), + }); + + writeln!( + ui.warning_default(), + r"The `fix.tool-command` config option is deprecated and will be removed in a future version." + )?; + writeln!( + ui.hint_default(), + r###"Replace it with the following: + [fix.tools.legacy-tool-command] + command = {} + patterns = ["all()"] + "###, + to_toml_value(&config.get::("fix.tool-command").unwrap()).unwrap() + )?; + } + if let Ok(tools_table) = config.get_table("fix.tools") { + // Convert the map into a sorted vector early so errors are deterministic. + let mut tools: Vec = tools_table + .into_iter() + .sorted_by(|a, b| a.0.cmp(&b.0)) + .map(|(_name, value)| -> Result { + let tool: RawToolConfig = value.try_deserialize()?; + Ok(ToolConfig { + command: tool.command, + matcher: FilesetExpression::union_all( + tool.patterns + .iter() + .map(|arg| { + fileset::parse( + arg, + &RepoPathUiConverter::Fs { + cwd: "".into(), + base: "".into(), + }, + ) + }) + .try_collect()?, + ) + .to_matcher(), + }) + }) + .try_collect()?; + tools_config.tools.append(&mut tools); + } + if tools_config.tools.is_empty() { + // TODO: This is not a useful message when one or both fields are present but + // have the wrong type. After removing `fix.tool-command`, it will be simpler to + // propagate any errors from `config.get_array("fix.tools")`. + Err(config_error( + "At least one entry of `fix.tools` or `fix.tool-command` is required.".to_string(), + )) + } else { + Ok(tools_config) + } +} diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index 79271de1e6..953d3908aa 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -506,7 +506,7 @@ } } }, - "fix": { + "fix": { "type": "object", "description": "Settings for jj fix", "properties": { @@ -515,7 +515,31 @@ "items": { "type": "string" }, - "description": "Shell command that takes file content on stdin and returns fixed file content on stdout" + "description": "Shell command that takes file content on stdin and returns fixed file content on stdout (deprecated)" + }, + "tools": { + "type": "object", + "additionalProperties": { + "type": "object", + "description": "Settings for how specific filesets are affected by a tool", + "properties": { + "command": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Arguments used to execute this tool" + }, + "patterns": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Filesets that will be affected by this tool" + } + } + }, + "description": "Settings for tools run by jj fix" } } } diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 7a0ecfe0ab..b06a2f5e0f 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -778,26 +778,60 @@ code formatting tools to revisions that may not be properly formatted yet. It can also be used to modify files with other tools like `sed` or `sort`. The changed files in the given revisions will be updated with any fixes -determined by passing their file content through the external tool. -Descendants will also be updated by passing their versions of the same files -through the same external tool, which will never result in new conflicts. -Files with existing conflicts will be updated on all sides of the conflict, -which can potentially increase or decrease the number of conflict markers. - -The external tool must accept the current file content on standard input, -and return the updated file content on standard output. The output will not -be used unless the tool exits with a successful exit code. Output on -standard error will be passed through to the terminal. - -The configuration schema is expected to change in the future. For now, it -defines a single command that will affect all changed files in the specified -revisions. For example, to format some Rust code changed in the working copy -revision, you could write this configuration: +determined by passing their file content through any external tools the user +has configured for those files. Descendants will also be updated by passing +their versions of the same files through the same tools, which will ensure +that the fixes are not lost. This will never result in new conflicts. Files +with existing conflicts will be updated on all sides of the conflict, which +can potentially increase or decrease the number of conflict markers. + +The external tools must accept the current file content on standard input, +and return the updated file content on standard output. A tool's output will +not be used unless it exits with a successful exit code. Output on standard +error will be passed through to the terminal. + +Tools are defined in a table where the keys are arbitrary identifiers and +the values have the following properties: + - `command`: The arguments used to run the tool. The first argument is the + path to an executable file. Arguments can contain the substring `$path`, + which will be replaced with the repo-relative path of the file being + fixed. It is useful to provide the path to tools that include the path in + error messages, or behave differently based on the directory or file + name. + - `patterns`: Determines which files the tool will affect. If this list is + empty, no files will be affected by the tool. If there are multiple + patterns, the tool is applied only once to each file in the union of the + patterns. + +For example, the following configuration defines how two code formatters +(`clang-format` and `black`) will apply to three different file extensions +(.cc, .h, and .py): + +[fix.tools.clang-format] +command = ["/usr/bin/clang-format", "--assume-filename=$path"] +patterns = ["glob:'**/*.cc'", + "glob:'**/*.h'"] + +[fix.tools.black] +command = ["/usr/bin/black", "-", "--stdin-filename=$path"] +patterns = ["glob:'**/*.py'"] + +Execution order of tools that affect the same file is deterministic, but +currently unspecified, and may change between releases. If two tools affect +the same file, the second tool to run will receive its input from the +output of the first tool. + +There is also a deprecated configuration schema that defines a single +command that will affect all changed files in the specified revisions. For +example, the following configuration would apply the Rust formatter to all +changed files (whether they are Rust files or not): [fix] tool-command = ["rustfmt", "--emit", "stdout"] -And then run the command `jj fix -s @`. +The tool defined by `tool-command` acts as if it was the first entry in +`fix.tools`, and uses `pattern = "all()"``. Support for `tool-command` +will be removed in a future version. **Usage:** `jj fix [OPTIONS] [PATHS]...` diff --git a/cli/tests/test_fix_command.rs b/cli/tests/test_fix_command.rs index b3153df3b3..da8e30b2f8 100644 --- a/cli/tests/test_fix_command.rs +++ b/cli/tests/test_fix_command.rs @@ -22,43 +22,415 @@ use jj_lib::file_util::try_symlink; use crate::common::TestEnvironment; /// Set up a repo where the `jj fix` command uses the fake editor with the given -/// flags. -fn init_with_fake_formatter(args: &[&str]) -> (TestEnvironment, PathBuf) { +/// flags. Returns a function that redacts the formatter executable's path from +/// a given string for test determinism. +fn init_with_fake_formatter(args: &[&str]) -> (TestEnvironment, PathBuf, impl Fn(&str) -> String) { let test_env = TestEnvironment::default(); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); assert!(formatter_path.is_file()); - let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + // The deprecated configuration syntax is still used by tests where it doesn't + // make a meaningful difference in coverage. Otherwise, we would have to add + // dedicated test coverage for the deprecated syntax until it is removed. We use + // single quotes here to avoid escaping issues when running the test on Windows. test_env.add_config(&format!( - r#"fix.tool-command = ["{}"]"#, - [escaped_formatter_path.as_str()] + r#"fix.tool-command = ['{}']"#, + [formatter_path.to_str().unwrap()] .iter() .chain(args) - .join(r#"", ""#) + .join(r#"', '"#) )); - (test_env, repo_path) + (test_env, repo_path, move |snapshot: &str| { + // When the test runs on Windows, backslashes in the path complicate things by + // changing the double quotes to single quotes in the serialized TOML. + snapshot.replace( + &if cfg!(windows) { + format!(r#"'{}'"#, formatter_path.to_str().unwrap()) + } else { + format!(r#""{}""#, formatter_path.to_str().unwrap()) + }, + "", + ) + }) } #[test] -fn test_fix_no_config() { +fn test_config_no_tools() { let test_env = TestEnvironment::default(); - test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); - let stderr = test_env.jj_cmd_failure(&repo_path, &["fix", "-s", "@"]); + + std::fs::write(repo_path.join("file"), "content\n").unwrap(); + let stderr = test_env.jj_cmd_failure(&repo_path, &["fix"]); insta::assert_snapshot!(stderr, @r###" - Config error: Invalid `fix.tool-command` - Caused by: configuration property "fix.tool-command" not found + Config error: At least one entry of `fix.tools` or `fix.tool-command` is required. For help, see https://github.com/martinvonz/jj/blob/main/docs/config.md. "###); + + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "@"]); + insta::assert_snapshot!(content, @"content\n"); +} + +#[test] +fn test_config_both_legacy_and_table_tools() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + + let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + test_env.add_config(&format!( + r###" + [fix] + tool-command = ["{formatter}", "--append", "legacy change"] + + [fix.tools.tool-1] + command = ["{formatter}", "--append", "tables change"] + patterns = ["tables-file"] + "###, + formatter = escaped_formatter_path.as_str() + )); + + std::fs::write(repo_path.join("legacy-file"), "legacy content\n").unwrap(); + std::fs::write(repo_path.join("tables-file"), "tables content\n").unwrap(); + + let (_stdout, _stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]); + + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "legacy-file", "-r", "@"]); + insta::assert_snapshot!(content, @r###" + legacy content + legacy change + "###); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "tables-file", "-r", "@"]); + insta::assert_snapshot!(content, @r###" + tables content + legacy change + tables change + "###); +} + +#[test] +fn test_config_multiple_tools() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + test_env.add_config(&format!( + r###" + [fix.tools.tool-1] + command = ["{formatter}", "--uppercase"] + patterns = ["foo"] + + [fix.tools.tool-2] + command = ["{formatter}", "--lowercase"] + patterns = ["bar"] + "###, + formatter = escaped_formatter_path.as_str() + )); + + std::fs::write(repo_path.join("foo"), "Foo\n").unwrap(); + std::fs::write(repo_path.join("bar"), "Bar\n").unwrap(); + std::fs::write(repo_path.join("baz"), "Baz\n").unwrap(); + + let (_stdout, _stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]); + + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo", "-r", "@"]); + insta::assert_snapshot!(content, @"FOO\n"); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "bar", "-r", "@"]); + insta::assert_snapshot!(content, @"bar\n"); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "baz", "-r", "@"]); + insta::assert_snapshot!(content, @"Baz\n"); +} + +#[test] +fn test_config_multiple_tools_with_same_name() { + let mut test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + + // Multiple definitions with the same `name` are not allowed, because it is + // likely to be a mistake, and mistakes are risky when they rewrite files. + test_env.add_config(&format!( + r###" + [fix.tools.my-tool] + command = ["{formatter}", "--uppercase"] + patterns = ["foo"] + + [fix.tools.my-tool] + command = ["{formatter}", "--lowercase"] + patterns = ["bar"] + "###, + formatter = escaped_formatter_path.as_str() + )); + + std::fs::write(repo_path.join("foo"), "Foo\n").unwrap(); + std::fs::write(repo_path.join("bar"), "Bar\n").unwrap(); + + let stderr = test_env.jj_cmd_failure(&repo_path, &["fix"]); + #[cfg(unix)] + insta::assert_snapshot!(stderr, @r###" + Config error: redefinition of table `fix.tools.my-tool` for key `fix.tools.my-tool` at line 6 column 9 in ../config/config0002.toml + For help, see https://github.com/martinvonz/jj/blob/main/docs/config.md. + "###); + #[cfg(windows)] + insta::assert_snapshot!(stderr, @r###" + Config error: redefinition of table `fix.tools.my-tool` for key `fix.tools.my-tool` at line 6 column 9 in ..\config\config0002.toml + For help, see https://github.com/martinvonz/jj/blob/main/docs/config.md. + "###); + + test_env.set_config_path("/dev/null".into()); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo", "-r", "@"]); + insta::assert_snapshot!(content, @"Foo\n"); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "bar", "-r", "@"]); + insta::assert_snapshot!(content, @"Bar\n"); +} + +#[test] +fn test_config_tables_overlapping_patterns() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + + test_env.add_config(&format!( + r###" + [fix.tools.tool-1] + command = ["{formatter}", "--append", "tool-1"] + patterns = ["foo", "bar"] + + [fix.tools.tool-2] + command = ["{formatter}", "--append", "tool-2"] + patterns = ["bar", "baz"] + "###, + formatter = escaped_formatter_path.as_str() + )); + + std::fs::write(repo_path.join("foo"), "foo\n").unwrap(); + std::fs::write(repo_path.join("bar"), "bar\n").unwrap(); + std::fs::write(repo_path.join("baz"), "baz\n").unwrap(); + + let (_stdout, _stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]); + + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo", "-r", "@"]); + insta::assert_snapshot!(content, @r###" + foo + tool-1 + "###); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "bar", "-r", "@"]); + insta::assert_snapshot!(content, @r###" + bar + tool-1 + tool-2 + "###); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "baz", "-r", "@"]); + insta::assert_snapshot!(content, @r###" + baz + tool-2 + "###); +} + +#[test] +fn test_config_tables_all_commands_missing() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + test_env.add_config( + r###" + [fix.tools.my-tool-missing-command-1] + patterns = ["foo"] + + [fix.tools.my-tool-missing-command-2] + patterns = ['glob:"ba*"'] + "###, + ); + + std::fs::write(repo_path.join("foo"), "foo\n").unwrap(); + + let stderr = test_env.jj_cmd_failure(&repo_path, &["fix"]); + insta::assert_snapshot!(stderr, @r###" + Config error: missing field `command` + For help, see https://github.com/martinvonz/jj/blob/main/docs/config.md. + "###); + + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo", "-r", "@"]); + insta::assert_snapshot!(content, @"foo\n"); +} + +#[test] +fn test_config_tables_some_commands_missing() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + test_env.add_config(&format!( + r###" + [fix.tools.tool-1] + command = ["{formatter}", "--uppercase"] + patterns = ["foo"] + + [fix.tools.my-tool-missing-command] + patterns = ['bar'] + "###, + formatter = escaped_formatter_path.as_str() + )); + + std::fs::write(repo_path.join("foo"), "foo\n").unwrap(); + + let stderr = test_env.jj_cmd_failure(&repo_path, &["fix"]); + insta::assert_snapshot!(stderr, @r###" + Config error: missing field `command` + For help, see https://github.com/martinvonz/jj/blob/main/docs/config.md. + "###); + + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo", "-r", "@"]); + insta::assert_snapshot!(content, @"foo\n"); +} + +#[test] +fn test_config_tables_empty_patterns_list() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + test_env.add_config(&format!( + r###" + [fix.tools.my-tool-empty-patterns] + command = ["{formatter}", "--uppercase"] + patterns = [] + "###, + formatter = escaped_formatter_path.as_str() + )); + + std::fs::write(repo_path.join("foo"), "foo\n").unwrap(); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Fixed 0 commits of 1 checked. + Nothing changed. + "###); + + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo", "-r", "@"]); + insta::assert_snapshot!(content, @"foo\n"); +} + +#[test] +fn test_config_filesets() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + test_env.add_config(&format!( + r###" + [fix.tools.my-tool-match-one] + command = ["{formatter}", "--uppercase"] + patterns = ['glob:"a*"'] + + [fix.tools.my-tool-match-two] + command = ["{formatter}", "--reverse"] + patterns = ['glob:"b*"'] + + [fix.tools.my-tool-match-none] + command = ["{formatter}", "--append", "SHOULD NOT APPEAR"] + patterns = ['glob:"this-doesnt-match-anything-*"'] + "###, + formatter = escaped_formatter_path.as_str() + )); + + std::fs::write(repo_path.join("a1"), "a1\n").unwrap(); + std::fs::write(repo_path.join("b1"), "b1\n").unwrap(); + std::fs::write(repo_path.join("b2"), "b2\n").unwrap(); + + let (_stdout, _stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]); + + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "a1", "-r", "@"]); + insta::assert_snapshot!(content, @"A1\n"); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "b1", "-r", "@"]); + insta::assert_snapshot!(content, @"1b\n"); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "b2", "-r", "@"]); + insta::assert_snapshot!(content, @"2b\n"); +} + +#[test] +fn test_relative_paths() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); + assert!(formatter_path.is_file()); + let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); + test_env.add_config(&format!( + r###" + [fix.tools.tool] + command = ["{formatter}", "--stdout", "Fixed!"] + patterns = ['glob:"foo*"'] + "###, + formatter = escaped_formatter_path.as_str() + )); + + std::fs::create_dir(repo_path.join("dir")).unwrap(); + std::fs::write(repo_path.join("foo1"), "unfixed\n").unwrap(); + std::fs::write(repo_path.join("foo2"), "unfixed\n").unwrap(); + std::fs::write(repo_path.join("dir/foo3"), "unfixed\n").unwrap(); + + // Positional arguments are cwd-relative, but the configured patterns are + // repo-relative, so this command fixes the empty intersection of those + // filesets. + let (_stdout, _stderr) = test_env.jj_cmd_ok(&repo_path.join("dir"), &["fix", "foo3"]); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo1", "-r", "@"]); + insta::assert_snapshot!(content, @"unfixed\n"); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo2", "-r", "@"]); + insta::assert_snapshot!(content, @"unfixed\n"); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "dir/foo3", "-r", "@"]); + insta::assert_snapshot!(content, @"unfixed\n"); + + // Positional arguments can specify a subset of the configured fileset. + let (_stdout, _stderr) = test_env.jj_cmd_ok(&repo_path.join("dir"), &["fix", "../foo1"]); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo1", "-r", "@"]); + insta::assert_snapshot!(content, @"Fixed!\n"); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo2", "-r", "@"]); + insta::assert_snapshot!(content, @"unfixed\n"); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "dir/foo3", "-r", "@"]); + insta::assert_snapshot!(content, @"unfixed\n"); + + // The current directory does not change the interpretation of the config, so + // foo2 is fixed but not dir/foo3. + let (_stdout, _stderr) = test_env.jj_cmd_ok(&repo_path.join("dir"), &["fix"]); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo1", "-r", "@"]); + insta::assert_snapshot!(content, @"Fixed!\n"); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo2", "-r", "@"]); + insta::assert_snapshot!(content, @"Fixed!\n"); + let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "dir/foo3", "-r", "@"]); + insta::assert_snapshot!(content, @"unfixed\n"); } #[test] fn test_fix_empty_commit() { - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Fixed 0 commits of 1 checked. Nothing changed. "###); @@ -66,14 +438,20 @@ fn test_fix_empty_commit() { #[test] fn test_fix_leaf_commit() { - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); std::fs::write(repo_path.join("file"), "unaffected").unwrap(); test_env.jj_cmd_ok(&repo_path, &["new"]); std::fs::write(repo_path.join("file"), "affected").unwrap(); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Fixed 1 commits of 1 checked. Working copy now at: rlvkpnrz 85ce8924 (no description set) Parent commit : qpvuntsm b2ca2bc5 (no description set) @@ -87,7 +465,7 @@ fn test_fix_leaf_commit() { #[test] fn test_fix_parent_commit() { - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); // Using one file name for all commits adds coverage of some possible bugs. std::fs::write(repo_path.join("file"), "parent").unwrap(); test_env.jj_cmd_ok(&repo_path, &["branch", "create", "parent"]); @@ -100,7 +478,13 @@ fn test_fix_parent_commit() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "parent"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Fixed 3 commits of 3 checked. Working copy now at: mzvwutvl d30c8ae2 child2 | (no description set) Parent commit : qpvuntsm 70a4dae2 parent | (no description set) @@ -116,7 +500,7 @@ fn test_fix_parent_commit() { #[test] fn test_fix_sibling_commit() { - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); std::fs::write(repo_path.join("file"), "parent").unwrap(); test_env.jj_cmd_ok(&repo_path, &["branch", "create", "parent"]); test_env.jj_cmd_ok(&repo_path, &["new"]); @@ -128,7 +512,13 @@ fn test_fix_sibling_commit() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "child1"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Fixed 1 commits of 1 checked. "###); let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "parent"]); @@ -141,7 +531,7 @@ fn test_fix_sibling_commit() { #[test] fn test_default_revset() { - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); std::fs::write(repo_path.join("file"), "trunk1").unwrap(); test_env.jj_cmd_ok(&repo_path, &["branch", "create", "trunk1"]); test_env.jj_cmd_ok(&repo_path, &["new"]); @@ -167,7 +557,13 @@ fn test_default_revset() { test_env.add_config(r#"revset-aliases."immutable_heads()" = "trunk2""#); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Fixed 3 commits of 3 checked. Working copy now at: yostqsxw dabc47b2 bar2 | (no description set) Parent commit : yqosqzyt 984b5924 bar1 | (no description set) @@ -189,7 +585,7 @@ fn test_default_revset() { #[test] fn test_custom_default_revset() { - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); std::fs::write(repo_path.join("file"), "foo").unwrap(); test_env.jj_cmd_ok(&repo_path, &["branch", "create", "foo"]); @@ -204,7 +600,13 @@ fn test_custom_default_revset() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Fixed 1 commits of 1 checked. "###); let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "file", "-r", "foo"]); @@ -215,7 +617,7 @@ fn test_custom_default_revset() { #[test] fn test_fix_immutable_commit() { - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); std::fs::write(repo_path.join("file"), "immutable").unwrap(); test_env.jj_cmd_ok(&repo_path, &["branch", "create", "immutable"]); test_env.jj_cmd_ok(&repo_path, &["new"]); @@ -224,7 +626,13 @@ fn test_fix_immutable_commit() { test_env.add_config(r#"revset-aliases."immutable_heads()" = "immutable""#); let stderr = test_env.jj_cmd_failure(&repo_path, &["fix", "-s", "immutable"]); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Error: Commit e4b41a3ce243 is immutable Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`. "###); @@ -236,12 +644,18 @@ fn test_fix_immutable_commit() { #[test] fn test_fix_empty_file() { - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); std::fs::write(repo_path.join("file"), "").unwrap(); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Fixed 0 commits of 1 checked. Nothing changed. "###); @@ -251,13 +665,19 @@ fn test_fix_empty_file() { #[test] fn test_fix_some_paths() { - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); std::fs::write(repo_path.join("file1"), "foo").unwrap(); std::fs::write(repo_path.join("file2"), "bar").unwrap(); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@", "file1"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Fixed 1 commits of 1 checked. Working copy now at: qpvuntsm 54a90d2b (no description set) Parent commit : zzzzzzzz 00000000 (empty) (no description set) @@ -273,12 +693,18 @@ fn test_fix_some_paths() { #[test] fn test_fix_cyclic() { - let (test_env, repo_path) = init_with_fake_formatter(&["--reverse"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--reverse"]); std::fs::write(repo_path.join("file"), "content\n").unwrap(); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--reverse"] + patterns = ["all()"] + Fixed 1 commits of 1 checked. Working copy now at: qpvuntsm bf5e6a5a (no description set) Parent commit : zzzzzzzz 00000000 (empty) (no description set) @@ -289,7 +715,13 @@ fn test_fix_cyclic() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--reverse"] + patterns = ["all()"] + Fixed 1 commits of 1 checked. Working copy now at: qpvuntsm 0e2d20d6 (no description set) Parent commit : zzzzzzzz 00000000 (empty) (no description set) @@ -304,7 +736,8 @@ fn test_deduplication() { // Append all fixed content to a log file. This assumes we're running the tool // in the root directory of the repo, which is worth reconsidering if we // establish a contract regarding cwd. - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase", "--tee", "$path-fixlog"]); + let (test_env, repo_path, redact) = + init_with_fake_formatter(&["--uppercase", "--tee", "$path-fixlog"]); // There are at least two interesting cases: the content is repeated immediately // in the child commit, or later in another descendant. @@ -322,7 +755,13 @@ fn test_deduplication() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "a"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase", "--tee", "$path-fixlog"] + patterns = ["all()"] + Fixed 4 commits of 4 checked. Working copy now at: yqosqzyt cf770245 d | (no description set) Parent commit : mzvwutvl 370615a5 c | (empty) (no description set) @@ -357,12 +796,18 @@ fn sorted_lines(path: PathBuf) -> String { fn test_executed_but_nothing_changed() { // Show that the tool ran by causing a side effect with --tee, and test that we // do the right thing when the tool's output is exactly equal to its input. - let (test_env, repo_path) = init_with_fake_formatter(&["--tee", "$path-copy"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--tee", "$path-copy"]); std::fs::write(repo_path.join("file"), "content\n").unwrap(); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--tee", "$path-copy"] + patterns = ["all()"] + Fixed 0 commits of 1 checked. Nothing changed. "###); @@ -374,12 +819,18 @@ fn test_executed_but_nothing_changed() { #[test] fn test_failure() { - let (test_env, repo_path) = init_with_fake_formatter(&["--fail"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--fail"]); std::fs::write(repo_path.join("file"), "content").unwrap(); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--fail"] + patterns = ["all()"] + Fixed 0 commits of 1 checked. Nothing changed. "###); @@ -389,7 +840,7 @@ fn test_failure() { #[test] fn test_stderr_success() { - let (test_env, repo_path) = + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--stderr", "error", "--stdout", "new content"]); std::fs::write(repo_path.join("file"), "old content").unwrap(); @@ -397,7 +848,13 @@ fn test_stderr_success() { // of passing it through directly. let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--stderr", "error", "--stdout", "new content"] + patterns = ["all()"] + errorFixed 1 commits of 1 checked. Working copy now at: qpvuntsm 487808ba (no description set) Parent commit : zzzzzzzz 00000000 (empty) (no description set) @@ -409,13 +866,19 @@ fn test_stderr_success() { #[test] fn test_stderr_failure() { - let (test_env, repo_path) = + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--stderr", "error", "--stdout", "new content", "--fail"]); std::fs::write(repo_path.join("file"), "old content").unwrap(); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--stderr", "error", "--stdout", "new content", "--fail"] + patterns = ["all()"] + errorFixed 0 commits of 1 checked. Nothing changed. "###); @@ -435,6 +898,12 @@ fn test_missing_command() { // support multiple tools, we should also keep going to see if any of the other // executions succeed. insta::assert_snapshot!(stderr, @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = ["this_executable_shouldnt_exist"] + patterns = ["all()"] + Fixed 0 commits of 1 checked. Nothing changed. "###); @@ -442,14 +911,20 @@ fn test_missing_command() { #[test] fn test_fix_file_types() { - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); std::fs::write(repo_path.join("file"), "content").unwrap(); std::fs::create_dir(repo_path.join("dir")).unwrap(); try_symlink("file", repo_path.join("link")).unwrap(); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Fixed 1 commits of 1 checked. Working copy now at: qpvuntsm 6836a9e4 (no description set) Parent commit : zzzzzzzz 00000000 (empty) (no description set) @@ -462,7 +937,7 @@ fn test_fix_file_types() { #[cfg(unix)] #[test] fn test_fix_executable() { - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); let path = repo_path.join("file"); std::fs::write(&path, "content").unwrap(); let mut permissions = std::fs::metadata(&path).unwrap().permissions(); @@ -471,7 +946,13 @@ fn test_fix_executable() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Fixed 1 commits of 1 checked. Working copy now at: qpvuntsm fee78e99 (no description set) Parent commit : zzzzzzzz 00000000 (empty) (no description set) @@ -487,7 +968,7 @@ fn test_fix_executable() { fn test_fix_trivial_merge_commit() { // All the changes are attributable to a parent, so none are fixed (in the same // way that none would be shown in `jj diff -r @`). - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); std::fs::write(repo_path.join("file_a"), "content a").unwrap(); std::fs::write(repo_path.join("file_c"), "content c").unwrap(); test_env.jj_cmd_ok(&repo_path, &["branch", "create", "a"]); @@ -499,7 +980,13 @@ fn test_fix_trivial_merge_commit() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Fixed 0 commits of 1 checked. Nothing changed. "###); @@ -515,7 +1002,7 @@ fn test_fix_trivial_merge_commit() { fn test_fix_adding_merge_commit() { // None of the changes are attributable to a parent, so they are all fixed (in // the same way that they would be shown in `jj diff -r @`). - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); std::fs::write(repo_path.join("file_a"), "content a").unwrap(); std::fs::write(repo_path.join("file_c"), "content c").unwrap(); test_env.jj_cmd_ok(&repo_path, &["branch", "create", "a"]); @@ -531,7 +1018,13 @@ fn test_fix_adding_merge_commit() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Fixed 1 commits of 1 checked. Working copy now at: mzvwutvl f93eb5a9 (no description set) Parent commit : qpvuntsm 6e64e7a7 a | (no description set) @@ -550,7 +1043,7 @@ fn test_fix_adding_merge_commit() { #[test] fn test_fix_both_sides_of_conflict() { - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); std::fs::write(repo_path.join("file"), "content a\n").unwrap(); test_env.jj_cmd_ok(&repo_path, &["branch", "create", "a"]); test_env.jj_cmd_ok(&repo_path, &["new", "@-"]); @@ -562,7 +1055,13 @@ fn test_fix_both_sides_of_conflict() { // fixed if we didn't fix the parents also. let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "a", "-s", "b"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Fixed 3 commits of 3 checked. Working copy now at: mzvwutvl 88866235 (conflict) (empty) (no description set) Parent commit : qpvuntsm 8e8aad69 a | (no description set) @@ -594,7 +1093,7 @@ fn test_fix_both_sides_of_conflict() { fn test_fix_resolve_conflict() { // If both sides of the conflict look the same after being fixed, the conflict // will be resolved. - let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]); + let (test_env, repo_path, redact) = init_with_fake_formatter(&["--uppercase"]); std::fs::write(repo_path.join("file"), "Content\n").unwrap(); test_env.jj_cmd_ok(&repo_path, &["branch", "create", "a"]); test_env.jj_cmd_ok(&repo_path, &["new", "@-"]); @@ -606,7 +1105,13 @@ fn test_fix_resolve_conflict() { // fixed if we didn't fix the parents also. let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "a", "-s", "b"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(redact(&stderr), @r###" + Warning: The `fix.tool-command` config option is deprecated and will be removed in a future version. + Hint: Replace it with the following: + [fix.tools.legacy-tool-command] + command = [, "--uppercase"] + patterns = ["all()"] + Fixed 3 commits of 3 checked. Working copy now at: mzvwutvl 50fd048d (empty) (no description set) Parent commit : qpvuntsm dd2721f1 a | (no description set) diff --git a/cli/tests/test_util_command.rs b/cli/tests/test_util_command.rs index 4e7502afa7..210db9523d 100644 --- a/cli/tests/test_util_command.rs +++ b/cli/tests/test_util_command.rs @@ -30,7 +30,7 @@ fn test_util_config_schema() { "description": "User configuration for Jujutsu VCS. See https://github.com/martinvonz/jj/blob/main/docs/config.md for details", "properties": { [...] - "fix": { + "fix": { [...] } } diff --git a/docs/config.md b/docs/config.md index 9788cc5dbb..5d2552004e 100644 --- a/docs/config.md +++ b/docs/config.md @@ -684,6 +684,66 @@ the conflict is done, `jj` assumes that the conflict was only partially resolved and parses the conflict markers to get the new state of the conflict. The conflict is considered fully resolved when there are no conflict markers left. +## Code formatting and other file content transformations + +The `jj fix` command allows you to efficiently rewrite files in complex commit +graphs with no risk of introducing conflicts, using tools like `clang-format` or +`prettier`. The tools run as subprocesses that take file content on standard +input and repeat it, with any desired changes, on standard output. The file is +only rewritten if the subprocess produces a successful exit code. + +### Enforce coding style rules + +Suppose you want to use `clang-format` to format your `*.c` and `*.h` files, +as well as sorting their `#include` directives. + +`jj fix` provides the file content anonymously on standard input, but the name +of the file being formatted may be important for include sorting or other output +like error messages. To address this, you can use the `$path` substitution to +provide the name of the file in a command argument. + +```toml +[fix.tools.clang-format] +command = ["/usr/bin/clang-format", "--sort-includes", "--assume-filename=$path"] +patterns = ["glob:'**/*.c'", + "glob:'**/*.h'"] +``` + +### Sort and remove duplicate lines from a file + +`jj fix` can also be used with tools that are not considered code formatters. + +Suppose you have a list of words in a text file in your repository, and you want +to keep the file sorted alphabetically and remove any duplicate words. + +```toml +[fix.tools.sort-word-list] +command = ["sort", "-u"] +patterns = ["word_list.txt"] +``` + +### Execution order of tools + +If two or more tools affect the same file, they are executed in the ascending +lexicographical order of their configured names. This will remain as a tie +breaker if other ordering mechanisms are introduced in the future. If you use +numbers in tool names to control execution order, remember to include enough +leading zeros so that, for example, `09` sorts before `10`. + +Suppose you want to keep only the 10 smallest numbers in a text file that +contains one number on each line. This can be accomplished with `sort` and +`head`, but execution order is important. + +```toml +[fix.tools.1-sort-numbers-file] +command = ["sort", "-n"] +patterns = ["numbers.txt"] + +[fix.tools.2-truncate-numbers-file] +command = ["head", "-n", "10"] +patterns = ["numbers.txt"] +``` + ## Commit Signing `jj` can be configured to sign and verify the commits it creates using either