Skip to content

Commit cbbc024

Browse files
committed
cli: port description template to templater
This implements a building block of "signed-off-by line" jj-vcs#1399 and "commit --verbose" jj-vcs#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 jj-vcs#1354
1 parent 98b3b82 commit cbbc024

File tree

11 files changed

+130
-40
lines changed

11 files changed

+130
-40
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
8282

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

85+
* [The default commit description template](docs/config.md#default-description)
86+
can now be configured by `templates.draft_commit_description`.
87+
8588
### Fixed bugs
8689

8790
* `jj diff --git` no longer shows the contents of binary files.

cli/src/cli_util.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1638,6 +1638,10 @@ impl WorkspaceCommandTransaction<'_> {
16381638
self.helper
16391639
}
16401640

1641+
pub fn settings(&self) -> &UserSettings {
1642+
&self.helper.settings
1643+
}
1644+
16411645
pub fn base_repo(&self) -> &Arc<ReadonlyRepo> {
16421646
self.tx.base_repo()
16431647
}

cli/src/commands/commit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ new working-copy commit.
111111
commit_builder.set_description(command.settings().default_description());
112112
}
113113
let temp_commit = commit_builder.write_hidden()?;
114-
let template = description_template(ui, tx.base_workspace_helper(), "", &temp_commit)?;
114+
let template = description_template(&tx, "", &temp_commit)?;
115115
edit_description(tx.base_repo(), &template, command.settings())?
116116
};
117117
commit_builder.set_description(description);

cli/src/commands/describe.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ pub(crate) fn cmd_describe(
9090
commit_builder.set_description(command.settings().default_description());
9191
}
9292
let temp_commit = commit_builder.write_hidden()?;
93-
let template = description_template(ui, tx.base_workspace_helper(), "", &temp_commit)?;
93+
let template = description_template(&tx, "", &temp_commit)?;
9494
edit_description(tx.base_repo(), &template, command.settings())?
9595
};
9696
commit_builder.set_description(description);

cli/src/commands/split.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,7 @@ the operation will be aborted.
133133
}
134134
let temp_commit = commit_builder.write_hidden()?;
135135
let template = description_template(
136-
ui,
137-
tx.base_workspace_helper(),
136+
&tx,
138137
"Enter a description for the first commit.",
139138
&temp_commit,
140139
)?;
@@ -176,8 +175,7 @@ the operation will be aborted.
176175
} else {
177176
let temp_commit = commit_builder.write_hidden()?;
178177
let template = description_template(
179-
ui,
180-
tx.base_workspace_helper(),
178+
&tx,
181179
"Enter a description for the second commit.",
182180
&temp_commit,
183181
)?;

cli/src/config/templates.toml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,18 @@ if(overridden,
2121
) ++ "\n"
2222
'''
2323

24+
# TODO: Provide hook point for diff customization (#1946)? We might want a
25+
# syntax to comment out full text diffs without using the "JJ: " prefix.
26+
draft_commit_description = '''
27+
concat(
28+
description,
29+
surround(
30+
"\nJJ: This commit contains the following changes:\n", "",
31+
indent("JJ: ", diff.summary()),
32+
),
33+
)
34+
'''
35+
2436
log = 'builtin_log_compact'
2537
op_log = 'builtin_op_log_compact'
2638
show = 'builtin_log_detailed'

cli/src/description_util.rs

Lines changed: 24 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
1+
use std::io::Write as _;
2+
3+
use bstr::ByteVec as _;
14
use itertools::Itertools;
25
use jj_lib::commit::Commit;
3-
use jj_lib::matchers::EverythingMatcher;
46
use jj_lib::repo::ReadonlyRepo;
57
use jj_lib::settings::UserSettings;
68

7-
use crate::cli_util::{edit_temp_file, WorkspaceCommandHelper};
9+
use crate::cli_util::{edit_temp_file, WorkspaceCommandTransaction};
810
use crate::command_error::CommandError;
9-
use crate::diff_util::DiffFormat;
1011
use crate::formatter::PlainTextFormatter;
1112
use crate::text_util;
12-
use crate::ui::Ui;
1313

1414
pub fn edit_description(
1515
repo: &ReadonlyRepo,
@@ -89,37 +89,30 @@ pub fn join_message_paragraphs(paragraphs: &[String]) -> String {
8989
.join("\n")
9090
}
9191

92+
/// Renders commit description template, which will be edited by user.
9293
pub fn description_template(
93-
ui: &Ui,
94-
workspace_command: &WorkspaceCommandHelper,
94+
tx: &WorkspaceCommandTransaction,
9595
intro: &str,
9696
commit: &Commit,
9797
) -> Result<String, CommandError> {
98-
let mut diff_summary_bytes = Vec::new();
99-
let diff_renderer = workspace_command.diff_renderer(vec![DiffFormat::Summary]);
100-
diff_renderer.show_patch(
101-
ui,
102-
&mut PlainTextFormatter::new(&mut diff_summary_bytes),
103-
commit,
104-
&EverythingMatcher,
105-
)?;
106-
let mut template_chunks = Vec::new();
98+
// TODO: Should "ui.default-description" be deprecated?
99+
// We might want default description templates per command instead. For
100+
// example, "backout_description" template will be rendered against the
101+
// commit to be backed out, and the generated description could be set
102+
// without spawning editor.
103+
104+
// Named as "draft" because the output can contain "JJ: " comment lines.
105+
let template_key = "templates.draft_commit_description";
106+
let template_text = tx.settings().config().get_string(template_key)?;
107+
let template = tx.parse_commit_template(&template_text)?;
108+
109+
let mut output = Vec::new();
107110
if !intro.is_empty() {
108-
template_chunks.push(format!("JJ: {intro}\n"));
111+
writeln!(output, "JJ: {intro}").unwrap();
109112
}
110-
template_chunks.push(commit.description().to_owned());
111-
if !diff_summary_bytes.is_empty() {
112-
template_chunks.push("\n".to_owned());
113-
template_chunks.push(diff_summary_to_description(&diff_summary_bytes));
114-
}
115-
Ok(template_chunks.concat())
116-
}
117-
118-
pub fn diff_summary_to_description(bytes: &[u8]) -> String {
119-
let text = std::str::from_utf8(bytes).expect(
120-
"Summary diffs and repo paths must always be valid UTF8.",
121-
// Double-check this assumption for diffs that include file content.
122-
);
123-
"JJ: This commit contains the following changes:\n".to_owned()
124-
+ &textwrap::indent(text, "JJ: ")
113+
template
114+
.format(commit, &mut PlainTextFormatter::new(&mut output))
115+
.expect("write() to vec backed formatter should never fail");
116+
// Template output is usually UTF-8, but it can contain file content.
117+
Ok(output.into_string_lossy())
125118
}

cli/tests/test_commit_command.rs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ fn test_commit_with_default_description() {
174174
175175
176176
TESTED=TODO
177+
177178
JJ: This commit contains the following changes:
178179
JJ: A file1
179180
JJ: A file2
@@ -182,6 +183,67 @@ fn test_commit_with_default_description() {
182183
"###);
183184
}
184185

186+
#[test]
187+
fn test_commit_with_description_template() {
188+
let mut test_env = TestEnvironment::default();
189+
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
190+
test_env.add_config(
191+
r#"
192+
[templates]
193+
draft_commit_description = '''
194+
concat(
195+
description,
196+
"\n",
197+
indent(
198+
"JJ: ",
199+
concat(
200+
"Author: " ++ format_detailed_signature(author) ++ "\n",
201+
"Committer: " ++ format_detailed_signature(committer) ++ "\n",
202+
"\n",
203+
diff.stat(76),
204+
),
205+
),
206+
)
207+
'''
208+
"#,
209+
);
210+
let workspace_path = test_env.env_root().join("repo");
211+
212+
let edit_script = test_env.set_up_fake_editor();
213+
std::fs::write(edit_script, ["dump editor"].join("\0")).unwrap();
214+
215+
std::fs::write(workspace_path.join("file1"), "foo\n").unwrap();
216+
std::fs::write(workspace_path.join("file2"), "bar\n").unwrap();
217+
218+
// Only file1 should be included in the diff
219+
test_env.jj_cmd_ok(&workspace_path, &["commit", "file1"]);
220+
insta::assert_snapshot!(
221+
std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(), @r###"
222+
223+
JJ: Author: Test User <[email protected]> (2001-02-03 08:05:08)
224+
JJ: Committer: Test User <[email protected]> (2001-02-03 08:05:08)
225+
226+
JJ: file1 | 1 +
227+
JJ: 1 file changed, 1 insertion(+), 0 deletions(-)
228+
229+
JJ: Lines starting with "JJ: " (like this one) will be removed.
230+
"###);
231+
232+
// Timestamp after the reset should be available to the template
233+
test_env.jj_cmd_ok(&workspace_path, &["commit", "--reset-author"]);
234+
insta::assert_snapshot!(
235+
std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(), @r###"
236+
237+
JJ: Author: Test User <[email protected]> (2001-02-03 08:05:09)
238+
JJ: Committer: Test User <[email protected]> (2001-02-03 08:05:09)
239+
240+
JJ: file2 | 1 +
241+
JJ: 1 file changed, 1 insertion(+), 0 deletions(-)
242+
243+
JJ: Lines starting with "JJ: " (like this one) will be removed.
244+
"###);
245+
}
246+
185247
#[test]
186248
fn test_commit_without_working_copy() {
187249
let test_env = TestEnvironment::default();

cli/tests/test_describe_command.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,7 @@ fn test_describe_default_description() {
271271
272272
273273
TESTED=TODO
274+
274275
JJ: This commit contains the following changes:
275276
JJ: A file1
276277
JJ: A file2

cli/tests/test_split_command.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ fn test_split_with_default_description() {
248248
249249
250250
TESTED=TODO
251+
251252
JJ: This commit contains the following changes:
252253
JJ: A file1
253254
@@ -366,6 +367,7 @@ fn test_split_siblings_no_descendants() {
366367
367368
368369
TESTED=TODO
370+
369371
JJ: This commit contains the following changes:
370372
JJ: A file1
371373

0 commit comments

Comments
 (0)