diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b976f2b98..9fd7368eda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,13 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). directly as positional arguments, such as `jj bisect --range=..main -- cargo check --all-targets`. +* `jj commit` now accepts `--stdin` to read the commit message from stdin, + matching the behavior of `jj describe`. This allows passing multi-line + messages without shell escaping issues. + +* `jj commit` now accepts `--edit` to open an editor after providing a message + via `--stdin` or `-m`, enabling two-stage message refinement. + ### Fixed bugs * `jj metaedit --author-timestamp` twice with the same value no longer diff --git a/cli/src/commands/commit.rs b/cli/src/commands/commit.rs index 3dd1430aab..f363062e3f 100644 --- a/cli/src/commands/commit.rs +++ b/cli/src/commands/commit.rs @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::io; +use std::io::Read as _; + use clap_complete::ArgValueCandidates; use clap_complete::ArgValueCompleter; use indoc::writedoc; @@ -61,8 +64,25 @@ pub(crate) struct CommitArgs { )] tool: Option, /// The change description to use (don't open editor) - #[arg(long = "message", short, value_name = "MESSAGE")] + #[arg( + long = "message", + short, + value_name = "MESSAGE", + conflicts_with = "stdin" + )] message_paragraphs: Vec, + /// Read the change description from stdin + /// + /// Diff selection runs before we read stdin, so console-based diff editors + /// remain usable. + #[arg(long)] + stdin: bool, + /// Open an editor + /// + /// Forces an editor to open when using `--stdin` or `--message` to + /// allow the message to be edited afterwards. + #[arg(long)] + edit: bool, /// Put these paths in the first commit #[arg( value_name = "FILESETS", @@ -167,18 +187,37 @@ new working-copy commit. commit_builder.set_author(new_author); } - let description = if !args.message_paragraphs.is_empty() { - let mut description = join_message_paragraphs(&args.message_paragraphs); - if !description.is_empty() { + let provided_description = if args.stdin { + let mut buffer = String::new(); + io::stdin().read_to_string(&mut buffer)?; + Some(buffer) + } else if !args.message_paragraphs.is_empty() { + Some(join_message_paragraphs(&args.message_paragraphs)) + } else { + None + }; + + let description = if let Some(mut description) = provided_description { + commit_builder.set_description(&description); + if args.edit || !description.is_empty() { // The first trailer would become the first line of the description. // Also, a commit with no description is treated in a special way in jujutsu: it // can be discarded as soon as it's no longer the working copy. Adding a - // trailer to an empty description would break that logic. - commit_builder.set_description(description); + // trailer to an empty description would break that logic unless the user + // explicitly opens the editor to review it. description = add_trailers(ui, &tx, &commit_builder)?; } - description + if args.edit { + commit_builder.set_description(&description); + let temp_commit = commit_builder.write_hidden()?; + let intro = ""; + let template = description_template(ui, &tx, intro, &temp_commit)?; + edit_description(&text_editor, &template)? + } else { + description + } } else { + // No description provided, must use editor let description = add_trailers(ui, &tx, &commit_builder)?; commit_builder.set_description(description); let temp_commit = commit_builder.write_hidden()?; diff --git a/cli/tests/test_commit_command.rs b/cli/tests/test_commit_command.rs index f112b03f48..0be70e606c 100644 --- a/cli/tests/test_commit_command.rs +++ b/cli/tests/test_commit_command.rs @@ -568,18 +568,14 @@ fn test_commit_trailers() { "); let editor0 = std::fs::read_to_string(test_env.env_root().join("editor0")).unwrap(); - insta::assert_snapshot!( - format!("-----\n{editor0}-----\n"), @r#" - ----- + insta::assert_snapshot!(editor0, @r#" +Reviewed-by: foo@bar.org - Reviewed-by: foo@bar.org - - JJ: Change ID: zsuskuln - JJ: - JJ: Lines starting with "JJ:" (like this one) will be removed. - ----- - "#); +JJ: Change ID: zsuskuln +JJ: +JJ: Lines starting with "JJ:" (like this one) will be removed. +"#); let output = work_dir.run_jj(["log", "--no-graph", "-r@-", "-Tdescription"]); insta::assert_snapshot!(output, @r" @@ -588,6 +584,92 @@ fn test_commit_trailers() { "); } +#[test] +fn test_commit_with_description_from_stdin() { + let test_env = TestEnvironment::default(); + test_env.run_jj_in(".", ["git", "init", "repo"]).success(); + let work_dir = test_env.work_dir("repo"); + + work_dir.write_file("file1", "foo\n"); + let output = work_dir + .run_jj_with(|cmd| { + cmd.args(["commit", "--stdin"]) + .write_stdin("Multi-line\n\ncommit message") + }) + .success(); + insta::assert_snapshot!(output); + + let output = work_dir.run_jj(["log", "--no-graph", "-r@-", "-Tdescription"]); + insta::assert_snapshot!(output); +} + +#[test] +fn test_commit_with_empty_description_from_stdin() { + let test_env = TestEnvironment::default(); + test_env.run_jj_in(".", ["git", "init", "repo"]).success(); + let work_dir = test_env.work_dir("repo"); + + work_dir.write_file("file1", "foo\n"); + let output = work_dir + .run_jj_with(|cmd| cmd.args(["commit", "--stdin"]).write_stdin("")) + .success(); + insta::assert_snapshot!(get_log_output(&work_dir)); + insta::assert_snapshot!(output); +} + +#[test] +fn test_commit_with_description_from_stdin_and_edit() { + let mut test_env = TestEnvironment::default(); + let edit_script = test_env.set_up_fake_editor(); + test_env.run_jj_in(".", ["git", "init", "repo"]).success(); + let work_dir = test_env.work_dir("repo"); + + work_dir.write_file("file1", "foo\n"); + std::fs::write(&edit_script, ["write\nedited message"].join("\0")).unwrap(); + work_dir + .run_jj_with(|cmd| { + cmd.args(["commit", "--stdin", "--edit"]) + .write_stdin("from stdin") + }) + .success(); + + let output = work_dir.run_jj(["log", "--no-graph", "-r@-", "-Tdescription"]); + insta::assert_snapshot!(output); +} + +#[test] +fn test_commit_stdin_conflicts_with_message() { + let test_env = TestEnvironment::default(); + test_env.run_jj_in(".", ["git", "init", "repo"]).success(); + let work_dir = test_env.work_dir("repo"); + + let output = work_dir.run_jj_with(|cmd| { + cmd.args(["commit", "--stdin", "-m", "message"]) + .write_stdin("stdin") + }); + insta::assert_snapshot!(output); +} + +#[test] +fn test_commit_stdin_with_interactive() { + let mut test_env = TestEnvironment::default(); + let diff_script = test_env.set_up_fake_diff_editor(); + test_env.run_jj_in(".", ["git", "init", "repo"]).success(); + let work_dir = test_env.work_dir("repo"); + + work_dir.write_file("file1", "foo\n"); + std::fs::write(&diff_script, "dump JJ-INSTRUCTIONS instrs").unwrap(); + let output = work_dir + .run_jj_with(|cmd| { + cmd.args(["commit", "--stdin", "--interactive"]) + .write_stdin("from stdin") + }) + .success(); + insta::assert_snapshot!(output); + let log_output = work_dir.run_jj(["log", "--no-graph", "-r@-", "-Tdescription"]); + insta::assert_snapshot!(log_output); +} + #[must_use] fn get_log_output(work_dir: &TestWorkDir) -> CommandOutput { let template = r#"commit_id.short() ++ " " ++ description"#;