From f422f1300cf13d42ffb3be369771d8fd028f5dce Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 26 Aug 2023 15:08:22 +0900 Subject: [PATCH] templater: move empty signature placeholder to user template This patch also extracts format_detailed_signature() function to deduplicate the "show" template bits. The added placeholder templates aren't labeled as "empty". If needed, I think the whole template can be labeled as "empty" (or "empty_commit") just like "working_copy". Closes #2112 --- CHANGELOG.md | 3 ++- cli/src/config/templates.toml | 15 +++++++++++---- cli/src/template_builder.rs | 20 ++++---------------- cli/src/templater.rs | 13 +++++-------- cli/tests/test_commit_template.rs | 2 +- cli/tests/test_templater.rs | 18 +++++++++--------- 6 files changed, 32 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dc26faf1..a04393bf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,7 +38,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 can be customized with the `snapshot.max-new-file-size` config option. * Author and committer signatures now use empty strings to represent unset - names and email addresses. + names and email addresses. The `author`/`committer` template keywords and + methods also return empty strings. Older binaries may not warn user when attempting to `git push` commits with such signatures. diff --git a/cli/src/config/templates.toml b/cli/src/config/templates.toml index aacc6247c..e1f535d7f 100644 --- a/cli/src/config/templates.toml +++ b/cli/src/config/templates.toml @@ -40,7 +40,7 @@ if(root, separate(" ", format_short_change_id(change_id) ++ if(divergent, "??"), if(hidden, "hidden"))), - author.username(), + if(author.email(), author.username(), email_placeholder), format_timestamp(committer.timestamp()), branches, tags, @@ -89,8 +89,8 @@ builtin_log_comfortable = 'builtin_log_compact ++ "\n"' concat( "Commit ID: " ++ commit_id ++ "\n", "Change ID: " ++ change_id ++ "\n", - "Author: " ++ author ++ " (" ++ format_timestamp(author.timestamp()) ++ ")\n", - "Committer: " ++ committer ++ " (" ++ format_timestamp(committer.timestamp()) ++ ")\n", + "Author: " ++ format_detailed_signature(author) ++ "\n", + "Committer: " ++ format_detailed_signature(committer) ++ "\n", "\n", indent(" ", if(description, description, description_placeholder ++ "\n")), "\n", @@ -123,13 +123,20 @@ separate(" ", description_placeholder = ''' label(if(empty, "empty ") ++ "description placeholder", "(no description set)")''' +email_placeholder = 'label("email placeholder", "(no email available)")' +name_placeholder = 'label("name placeholder", "(no name available)")' commit_summary_separator = 'label("separator", " | ")' # Hook points for users to customize the default templates: 'format_short_id(id)' = 'id.shortest(8)' 'format_short_change_id(id)' = 'format_short_id(id)' 'format_short_commit_id(id)' = 'format_short_id(id)' -'format_short_signature(signature)' = 'signature.email()' +'format_short_signature(signature)' = ''' + if(signature.email(), signature.email(), email_placeholder)''' +'format_detailed_signature(signature)' = ''' + if(signature.name(), signature.name(), name_placeholder) + ++ " <" ++ if(signature.email(), signature.email(), email_placeholder) ++ ">" + ++ " (" ++ format_timestamp(signature.timestamp()) ++ ")"''' 'format_time_range(time_range)' = ''' time_range.start().ago() ++ label("time", ", lasted ") ++ time_range.duration()''' 'format_timestamp(timestamp)' = 'timestamp' diff --git a/cli/src/template_builder.rs b/cli/src/template_builder.rs index 1de4bdaaa..3032bad51 100644 --- a/cli/src/template_builder.rs +++ b/cli/src/template_builder.rs @@ -456,32 +456,20 @@ fn build_signature_method<'a, L: TemplateLanguage<'a>>( "name" => { template_parser::expect_no_arguments(function)?; language.wrap_string(TemplateFunction::new(self_property, |signature| { - if !signature.name.is_empty() { - signature.name - } else { - "(no name available)".to_string() - } + signature.name })) } "email" => { template_parser::expect_no_arguments(function)?; language.wrap_string(TemplateFunction::new(self_property, |signature| { - if !signature.email.is_empty() { - signature.email - } else { - "(no email available)".to_string() - } + signature.email })) } "username" => { template_parser::expect_no_arguments(function)?; language.wrap_string(TemplateFunction::new(self_property, |signature| { - if !signature.email.is_empty() { - let (username, _) = text_util::split_email(&signature.email); - username.to_owned() - } else { - "(no username available)".to_string() - } + let (username, _) = text_util::split_email(&signature.email); + username.to_owned() })) } "timestamp" => { diff --git a/cli/src/templater.rs b/cli/src/templater.rs index 30c909355..623692e6d 100644 --- a/cli/src/templater.rs +++ b/cli/src/templater.rs @@ -57,18 +57,15 @@ impl + ?Sized> Template for Box { impl Template<()> for Signature { fn format(&self, _: &(), formatter: &mut dyn Formatter) -> io::Result<()> { - if !self.name.is_empty() { - write!(formatter.labeled("name"), "{}", self.name)?; - } else { - write!(formatter.labeled("name"), "(no name available)")?; + write!(formatter.labeled("name"), "{}", self.name)?; + if !self.name.is_empty() && !self.email.is_empty() { + write!(formatter, " ")?; } - write!(formatter, " <")?; if !self.email.is_empty() { + write!(formatter, "<")?; write!(formatter.labeled("email"), "{}", self.email)?; - } else { - write!(formatter.labeled("email"), "(no email available)")?; + write!(formatter, ">")?; } - write!(formatter, ">")?; Ok(()) } } diff --git a/cli/tests/test_commit_template.rs b/cli/tests/test_commit_template.rs index c11d69211..d6a9761e4 100644 --- a/cli/tests/test_commit_template.rs +++ b/cli/tests/test_commit_template.rs @@ -182,7 +182,7 @@ fn test_log_builtin_templates() { ); insta::assert_snapshot!(render(r#"builtin_log_oneline"#), @r###" - @ rlvkpnrz (no username available) 2001-02-03 04:05:08.000 +07:00 dc315397 (empty) (no description set) + @ rlvkpnrz (no email available) 2001-02-03 04:05:08.000 +07:00 dc315397 (empty) (no description set) ◉ qpvuntsm test.user 2001-02-03 04:05:07.000 +07:00 230dd059 (empty) (no description set) ◉ zzzzzzzz root 00000000 "###); diff --git a/cli/tests/test_templater.rs b/cli/tests/test_templater.rs index 022c9df39..b3b89429b 100644 --- a/cli/tests/test_templater.rs +++ b/cli/tests/test_templater.rs @@ -463,17 +463,17 @@ fn test_templater_signature() { test_env.jj_cmd_ok(&repo_path, &["--config-toml=user.name=''", "new"]); - insta::assert_snapshot!(render(r#"author"#), @"(no name available) "); - insta::assert_snapshot!(render(r#"author.name()"#), @"(no name available)"); + insta::assert_snapshot!(render(r#"author"#), @""); + insta::assert_snapshot!(render(r#"author.name()"#), @""); insta::assert_snapshot!(render(r#"author.email()"#), @"test.user@example.com"); insta::assert_snapshot!(render(r#"author.username()"#), @"test.user"); test_env.jj_cmd_ok(&repo_path, &["--config-toml=user.email=''", "new"]); - insta::assert_snapshot!(render(r#"author"#), @"Test User <(no email available)>"); + insta::assert_snapshot!(render(r#"author"#), @"Test User"); insta::assert_snapshot!(render(r#"author.name()"#), @"Test User"); - insta::assert_snapshot!(render(r#"author.email()"#), @"(no email available)"); - insta::assert_snapshot!(render(r#"author.username()"#), @"(no username available)"); + insta::assert_snapshot!(render(r#"author.email()"#), @""); + insta::assert_snapshot!(render(r#"author.username()"#), @""); test_env.jj_cmd_ok( &repo_path, @@ -484,10 +484,10 @@ fn test_templater_signature() { ], ); - insta::assert_snapshot!(render(r#"author"#), @"(no name available) <(no email available)>"); - insta::assert_snapshot!(render(r#"author.name()"#), @"(no name available)"); - insta::assert_snapshot!(render(r#"author.email()"#), @"(no email available)"); - insta::assert_snapshot!(render(r#"author.username()"#), @"(no username available)"); + insta::assert_snapshot!(render(r#"author"#), @""); + insta::assert_snapshot!(render(r#"author.name()"#), @""); + insta::assert_snapshot!(render(r#"author.email()"#), @""); + insta::assert_snapshot!(render(r#"author.username()"#), @""); } #[test]