Skip to content

Commit

Permalink
cli: port description template to templater
Browse files Browse the repository at this point in the history
This implements a building block of "signed-off-by line" #1399 and "commit
--verbose" #1946. We'll probably need an easy way to customize the diff part,
but I'm not sure if it can be as simple as a template alias function. User
might want to embed diffs without "JJ: " prefixes?

Perhaps, we can deprecate "ui.default-description", but it's not addressed in
this patch. It could be replaced with "default_description" template alias,
but we might want to configure default per command. Suppose we add a default
"backout_description" template, it would have to be rendered against the
source commit, not the newly-created backout commit.

The template key is named as "draft_commit_description" because it is the
template to generate an editor template. "templates.commit_description_template"
sounds a bit odd.

There's one minor behavior change: the default description is now terminated
by "\n".

Closes #1354
  • Loading branch information
yuja committed Jul 25, 2024
1 parent 5a19eb6 commit d6e9788
Show file tree
Hide file tree
Showing 11 changed files with 130 additions and 40 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

* New config setting `git.private-commits` to prevent commits from being pushed.

* [The default commit description template](docs/config.md#default-description)
can now be configured by `templates.draft_commit_description`.

### Fixed bugs

* `jj diff --git` no longer shows the contents of binary files.
Expand Down
4 changes: 4 additions & 0 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1638,6 +1638,10 @@ impl WorkspaceCommandTransaction<'_> {
self.helper
}

pub fn settings(&self) -> &UserSettings {
&self.helper.settings
}

pub fn base_repo(&self) -> &Arc<ReadonlyRepo> {
self.tx.base_repo()
}
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ new working-copy commit.
commit_builder.set_description(command.settings().default_description());
}
let temp_commit = commit_builder.write_hidden()?;
let template = description_template(ui, tx.base_workspace_helper(), "", &temp_commit)?;
let template = description_template(&tx, "", &temp_commit)?;
edit_description(tx.base_repo(), &template, command.settings())?
};
commit_builder.set_description(description);
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/describe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ pub(crate) fn cmd_describe(
commit_builder.set_description(command.settings().default_description());
}
let temp_commit = commit_builder.write_hidden()?;
let template = description_template(ui, tx.base_workspace_helper(), "", &temp_commit)?;
let template = description_template(&tx, "", &temp_commit)?;
edit_description(tx.base_repo(), &template, command.settings())?
};
commit_builder.set_description(description);
Expand Down
6 changes: 2 additions & 4 deletions cli/src/commands/split.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@ the operation will be aborted.
}
let temp_commit = commit_builder.write_hidden()?;
let template = description_template(
ui,
tx.base_workspace_helper(),
&tx,
"Enter a description for the first commit.",
&temp_commit,
)?;
Expand Down Expand Up @@ -176,8 +175,7 @@ the operation will be aborted.
} else {
let temp_commit = commit_builder.write_hidden()?;
let template = description_template(
ui,
tx.base_workspace_helper(),
&tx,
"Enter a description for the second commit.",
&temp_commit,
)?;
Expand Down
12 changes: 12 additions & 0 deletions cli/src/config/templates.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ if(overridden,
) ++ "\n"
'''

# TODO: Provide hook point for diff customization (#1946)? We might want a
# syntax to comment out full text diffs without using the "JJ: " prefix.
draft_commit_description = '''
concat(
description,
surround(
"\nJJ: This commit contains the following changes:\n", "",
indent("JJ: ", diff.summary()),
),
)
'''

log = 'builtin_log_compact'
op_log = 'builtin_op_log_compact'
show = 'builtin_log_detailed'
Expand Down
55 changes: 24 additions & 31 deletions cli/src/description_util.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use std::io::Write as _;

use bstr::ByteVec as _;
use itertools::Itertools;
use jj_lib::commit::Commit;
use jj_lib::matchers::EverythingMatcher;
use jj_lib::repo::ReadonlyRepo;
use jj_lib::settings::UserSettings;

use crate::cli_util::{edit_temp_file, WorkspaceCommandHelper};
use crate::cli_util::{edit_temp_file, WorkspaceCommandTransaction};
use crate::command_error::CommandError;
use crate::diff_util::DiffFormat;
use crate::formatter::PlainTextFormatter;
use crate::text_util;
use crate::ui::Ui;

pub fn edit_description(
repo: &ReadonlyRepo,
Expand Down Expand Up @@ -89,37 +89,30 @@ pub fn join_message_paragraphs(paragraphs: &[String]) -> String {
.join("\n")
}

/// Renders commit description template, which will be edited by user.
pub fn description_template(
ui: &Ui,
workspace_command: &WorkspaceCommandHelper,
tx: &WorkspaceCommandTransaction,
intro: &str,
commit: &Commit,
) -> Result<String, CommandError> {
let mut diff_summary_bytes = Vec::new();
let diff_renderer = workspace_command.diff_renderer(vec![DiffFormat::Summary]);
diff_renderer.show_patch(
ui,
&mut PlainTextFormatter::new(&mut diff_summary_bytes),
commit,
&EverythingMatcher,
)?;
let mut template_chunks = Vec::new();
// TODO: Should "ui.default-description" be deprecated?
// We might want default description templates per command instead. For
// example, "backout_description" template will be rendered against the
// 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.
let template_key = "templates.draft_commit_description";
let template_text = tx.settings().config().get_string(template_key)?;
let template = tx.parse_commit_template(&template_text)?;

let mut output = Vec::new();
if !intro.is_empty() {
template_chunks.push(format!("JJ: {intro}\n"));
writeln!(output, "JJ: {intro}").unwrap();
}
template_chunks.push(commit.description().to_owned());
if !diff_summary_bytes.is_empty() {
template_chunks.push("\n".to_owned());
template_chunks.push(diff_summary_to_description(&diff_summary_bytes));
}
Ok(template_chunks.concat())
}

pub fn diff_summary_to_description(bytes: &[u8]) -> String {
let text = std::str::from_utf8(bytes).expect(
"Summary diffs and repo paths must always be valid UTF8.",
// Double-check this assumption for diffs that include file content.
);
"JJ: This commit contains the following changes:\n".to_owned()
+ &textwrap::indent(text, "JJ: ")
template
.format(commit, &mut PlainTextFormatter::new(&mut output))
.expect("write() to vec backed formatter should never fail");
// Template output is usually UTF-8, but it can contain file content.
Ok(output.into_string_lossy())
}
62 changes: 62 additions & 0 deletions cli/tests/test_commit_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ fn test_commit_with_default_description() {
TESTED=TODO
JJ: This commit contains the following changes:
JJ: A file1
JJ: A file2
Expand All @@ -182,6 +183,67 @@ fn test_commit_with_default_description() {
"###);
}

#[test]
fn test_commit_with_description_template() {
let mut test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
test_env.add_config(
r#"
[templates]
draft_commit_description = '''
concat(
description,
"\n",
indent(
"JJ: ",
concat(
"Author: " ++ format_detailed_signature(author) ++ "\n",
"Committer: " ++ format_detailed_signature(committer) ++ "\n",
"\n",
diff.stat(76),
),
),
)
'''
"#,
);
let workspace_path = test_env.env_root().join("repo");

let edit_script = test_env.set_up_fake_editor();
std::fs::write(edit_script, ["dump editor"].join("\0")).unwrap();

std::fs::write(workspace_path.join("file1"), "foo\n").unwrap();
std::fs::write(workspace_path.join("file2"), "bar\n").unwrap();

// Only file1 should be included in the diff
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.
"###);

// 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###"
JJ: Author: Test User <[email protected]> (2001-02-03 08:05:09)
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.
"###);
}

#[test]
fn test_commit_without_working_copy() {
let test_env = TestEnvironment::default();
Expand Down
1 change: 1 addition & 0 deletions cli/tests/test_describe_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ fn test_describe_default_description() {
TESTED=TODO
JJ: This commit contains the following changes:
JJ: A file1
JJ: A file2
Expand Down
2 changes: 2 additions & 0 deletions cli/tests/test_split_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ fn test_split_with_default_description() {
TESTED=TODO
JJ: This commit contains the following changes:
JJ: A file1
Expand Down Expand Up @@ -366,6 +367,7 @@ fn test_split_siblings_no_descendants() {
TESTED=TODO
JJ: This commit contains the following changes:
JJ: A file1
Expand Down
21 changes: 18 additions & 3 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,24 @@ ui.default-command = ["log", "--reversed"]

### Default description

The value of the `ui.default-description` setting will be used to prepopulate
the editor when describing changes with an empty description. This could be a
useful reminder to fill in things like BUG=, TESTED= etc.
The editor content of a commit description can be populated by the
`draft_commit_description` template.

```toml
[templates]
draft_commit_description = '''
concat(
description,
surround(
"\nJJ: This commit contains the following changes:\n", "",
indent("JJ: ", diff.stat(72)),
),
)
'''
```

The value of the `ui.default-description` setting can also be used in order to
fill in things like BUG=, TESTED= etc.

```toml
ui.default-description = "\n\nTESTED=TODO"
Expand Down

0 comments on commit d6e9788

Please sign in to comment.