Skip to content

Commit

Permalink
cli: consider "JJ:" lines as comments also when not followed by space
Browse files Browse the repository at this point in the history
We currently ignore lines prefixed with "JJ: " (including the space)
in commit messages and in the list of sparse paths from `jj sparse
edit`. I think I included the trailing space in the prefix simply
because that's how we render comments line we insert before we ask the
user to edit the file. However, as #5004 says, Git doesn't require a
space after their "#" prefix. Neither does Mercurial after their "HG:"
prefix. So let's follow their lead and not require the trailing
space. Seems useful especially for people who have their editor
configured to strip trailing spaces.
  • Loading branch information
martinvonz committed Dec 4, 2024
1 parent 94c6d6e commit 043676a
Show file tree
Hide file tree
Showing 9 changed files with 37 additions and 41 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
* `jj git push` no longer pushes new bookmarks by default. Use `--allow-new` to
bypass this restriction.

* Lines prefixed with "JJ:" in commit descriptions and in sparse patterns (from
`jj sparse edit`) are now stripped even if they are not immediately followed
by a space. [#5004](https://github.com/martinvonz/jj/issues/5004)

### Deprecations

### New features
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/sparse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ fn edit_sparse(

content
.lines()
.filter(|line| !line.starts_with("JJ: "))
.filter(|line| !line.starts_with("JJ:"))
.map(|line| line.trim())
.filter(|line| !line.is_empty())
.map(|line| {
Expand Down
2 changes: 1 addition & 1 deletion cli/src/config/templates.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ if(overridden,
'''

# TODO: Provide hook point for diff customization (#1946)? We might want a
# syntax to comment out full text diffs without using the "JJ: " prefix.
# syntax to comment out full text diffs without using the "JJ:" prefix.
draft_commit_description = '''
concat(
description,
Expand Down
8 changes: 4 additions & 4 deletions cli/src/description_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ where
{
let description = lines
.into_iter()
.filter(|line| !line.as_ref().starts_with("JJ: "))
.filter(|line| !line.as_ref().starts_with("JJ:"))
.fold(String::new(), |acc, line| acc + line.as_ref() + "\n");
text_util::complete_newline(description.trim_matches('\n'))
}
Expand All @@ -40,7 +40,7 @@ pub fn edit_description(
) -> Result<String, CommandError> {
let description = format!(
r#"{description}
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"#
);

Expand Down Expand Up @@ -138,7 +138,7 @@ where
lines.push(line);
}
// Do not allow lines without a commit header, except for empty lines or comments.
else if !line.trim().is_empty() && !line.starts_with("JJ: ") {
else if !line.trim().is_empty() && !line.starts_with("JJ:") {
return Err(ParseBulkEditMessageError::LineWithoutCommitHeader(
line.to_owned(),
));
Expand Down Expand Up @@ -237,7 +237,7 @@ pub fn description_template(
// commit to be backed out, and the generated description could be set
// without spawning editor.

// Named as "draft" because the output can contain "JJ: " comment lines.
// Named as "draft" because the output can contain "JJ:" comment lines.
let template_key = "templates.draft_commit_description";
let template_text = tx.settings().get_string(template_key)?;
let template = tx.parse_commit_template(ui, &template_text)?;
Expand Down
29 changes: 12 additions & 17 deletions cli/tests/test_commit_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ fn test_commit_with_editor() {
std::fs::read_to_string(test_env.env_root().join("editor0")).unwrap(), @r###"
initial
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);

// Check that the editor content includes diff summary
Expand All @@ -70,7 +70,7 @@ fn test_commit_with_editor() {
JJ: A file1
JJ: A file2
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);
}

Expand Down Expand Up @@ -127,7 +127,7 @@ fn test_commit_interactive() {
JJ: This commit contains the following changes:
JJ: A file1
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);

// Try again with --tool=<name>, which implies --interactive
Expand All @@ -148,7 +148,7 @@ fn test_commit_interactive() {
JJ: This commit contains the following changes:
JJ: A file1
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);
}

Expand All @@ -172,15 +172,13 @@ fn test_commit_with_default_description() {
"###);
insta::assert_snapshot!(
std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(), @r###"
TESTED=TODO
JJ: This commit contains the following changes:
JJ: A file1
JJ: A file2
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);
}

Expand Down Expand Up @@ -221,14 +219,13 @@ fn test_commit_with_description_template() {
test_env.jj_cmd_ok(&workspace_path, &["commit", "file1"]);
insta::assert_snapshot!(
std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(), @r###"
JJ: Author: Test User <[email protected]> (2001-02-03 08:05:08)
JJ: Committer: Test User <[email protected]> (2001-02-03 08:05:08)
JJ: file1 | 1 +
JJ: 1 file changed, 1 insertion(+), 0 deletions(-)
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);

// Only file2 with modified author should be included in the diff
Expand All @@ -242,30 +239,28 @@ fn test_commit_with_description_template() {
],
);
insta::assert_snapshot!(
std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(), @r#"
std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(), @r###"
JJ: Author: Another User <[email protected]> (2001-02-03 08:05:08)
JJ: Committer: Test User <[email protected]> (2001-02-03 08:05:09)
JJ: file2 | 1 +
JJ: 1 file changed, 1 insertion(+), 0 deletions(-)
JJ: Lines starting with "JJ: " (like this one) will be removed.
"#);
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);

// Timestamp after the reset should be available to the template
test_env.jj_cmd_ok(&workspace_path, &["commit", "--reset-author"]);
insta::assert_snapshot!(
std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(), @r#"
std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(), @r###"
JJ: Author: Test User <[email protected]> (2001-02-03 08:05:10)
JJ: Committer: Test User <[email protected]> (2001-02-03 08:05:10)
JJ: file3 | 1 +
JJ: 1 file changed, 1 insertion(+), 0 deletions(-)
JJ: Lines starting with "JJ: " (like this one) will be removed.
"#);
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);
}

#[test]
Expand Down
13 changes: 5 additions & 8 deletions cli/tests/test_describe_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn test_describe() {
std::fs::read_to_string(test_env.env_root().join("editor0")).unwrap(), @r###"
description from CLI
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);

// Set a description in editor
Expand Down Expand Up @@ -475,15 +475,13 @@ fn test_describe_default_description() {
"###);
insta::assert_snapshot!(
std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(), @r###"
TESTED=TODO
JJ: This commit contains the following changes:
JJ: A file1
JJ: A file2
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);
}

Expand Down Expand Up @@ -566,15 +564,14 @@ fn test_describe_author() {
~
"#);
insta::assert_snapshot!(
std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(), @r#"
std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(), @r###"
JJ: Author: Super Seeder <[email protected]> (2001-02-03 08:05:12)
JJ: Committer: Test User <[email protected]> (2001-02-03 08:05:12)
JJ: 0 files changed, 0 insertions(+), 0 deletions(-)
JJ: Lines starting with "JJ: " (like this one) will be removed.
"#);
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);

// Change the author for multiple commits (the committer is always reset)
test_env.jj_cmd_ok(
Expand Down
16 changes: 8 additions & 8 deletions cli/tests/test_split_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ fn test_split_by_paths() {
JJ: This commit contains the following changes:
JJ: A file2
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);
assert!(!test_env.env_root().join("editor1").exists());

Expand Down Expand Up @@ -192,7 +192,7 @@ fn test_split_with_non_empty_description() {
JJ: This commit contains the following changes:
JJ: A file1
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);
insta::assert_snapshot!(
std::fs::read_to_string(test_env.env_root().join("editor2")).unwrap(), @r###"
Expand All @@ -202,7 +202,7 @@ fn test_split_with_non_empty_description() {
JJ: This commit contains the following changes:
JJ: A file2
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);
insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
@ kkmpptxzrspx false part 2
Expand Down Expand Up @@ -254,7 +254,7 @@ fn test_split_with_default_description() {
JJ: This commit contains the following changes:
JJ: A file1
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);
assert!(!test_env.env_root().join("editor2").exists());
insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
Expand Down Expand Up @@ -373,7 +373,7 @@ fn test_split_siblings_no_descendants() {
JJ: This commit contains the following changes:
JJ: A file1
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);
assert!(!test_env.env_root().join("editor2").exists());
}
Expand Down Expand Up @@ -451,7 +451,7 @@ fn test_split_siblings_with_descendants() {
JJ: This commit contains the following changes:
JJ: A file1
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);
insta::assert_snapshot!(
std::fs::read_to_string(test_env.env_root().join("editor2")).unwrap(), @r###"
Expand All @@ -461,7 +461,7 @@ fn test_split_siblings_with_descendants() {
JJ: This commit contains the following changes:
JJ: A file2
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);
}

Expand Down Expand Up @@ -593,7 +593,7 @@ fn test_split_interactive() {
JJ: This commit contains the following changes:
JJ: A file1
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);

insta::assert_snapshot!(stdout, @"");
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/test_squash_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1074,7 +1074,7 @@ fn test_squash_description() {
JJ: Description from source commit:
source
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);

// An explicit description on the command-line overrides prevents launching an
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/test_unsquash_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ fn test_unsquash_description() {
JJ: Description from source commit:
source
JJ: Lines starting with "JJ: " (like this one) will be removed.
JJ: Lines starting with "JJ:" (like this one) will be removed.
"###);

// If the source's *content* doesn't become empty, then the source remains and
Expand Down

0 comments on commit 043676a

Please sign in to comment.