Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 46 additions & 7 deletions cli/src/commands/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -61,8 +64,25 @@ pub(crate) struct CommitArgs {
)]
tool: Option<String>,
/// 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<String>,
/// 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",
Expand Down Expand Up @@ -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()?;
Expand Down
102 changes: 92 additions & 10 deletions cli/tests/test_commit_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: [email protected]

Reviewed-by: [email protected]

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.
"#);
Comment on lines -571 to +578
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert this change. I think the intent here is to ensure that the editor content is terminated by \n. insta::assert_snapshot!() isn't strict about leading/trailing newlines.


let output = work_dir.run_jj(["log", "--no-graph", "-r@-", "-Tdescription"]);
insta::assert_snapshot!(output, @r"
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use inline version: insta::assert_snapshot!(output, @"");

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you forgot to upload the latest change?


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"#;
Expand Down
Loading