From f30ac35ce3e72b9e04a6284badc5305d6be57274 Mon Sep 17 00:00:00 2001 From: Benjamin Tan Date: Thu, 12 Dec 2024 02:25:32 +0800 Subject: [PATCH] templater: add Email template type, deprecate `Signature.username()` The `Signature.email()` method is also updated to return the new Email type. The `Signature.username()` method is deprecated for `Signature.email().local()`. --- CHANGELOG.md | 6 ++++ cli/src/config/templates.toml | 4 +-- cli/src/template_builder.rs | 56 +++++++++++++++++++++++++++++-- cli/src/templater.rs | 31 ++++++++++++++++- cli/tests/test_commit_template.rs | 10 +++--- cli/tests/test_log_command.rs | 2 +- cli/tests/test_show_command.rs | 8 ++--- cli/tests/test_templater.rs | 10 +++++- docs/templates.md | 10 ++++-- 9 files changed, 118 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dbcb57d60..06b6aacc9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Deprecations +* The `Signature.username()` template method is deprecated for + `Signature().email().local()`. + ### New features * `jj` command no longer fails due to new working-copy files larger than the @@ -38,6 +41,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * Templates now support the `>=`, `>`, `<=`, and `<` relational operators for `Integer` types. +* A new Email template type is added. `Signature.email()` now returns an Email + template type instead of a String. + ### Fixed bugs * The `$NO_COLOR` environment variable must now be non-empty to be respected. diff --git a/cli/src/config/templates.toml b/cli/src/config/templates.toml index 04c4cda8aa..eae445cf1b 100644 --- a/cli/src/config/templates.toml +++ b/cli/src/config/templates.toml @@ -17,7 +17,7 @@ commit_summary = 'format_commit_summary_with_refs(self, bookmarks)' annotate_commit_summary = ''' separate(" ", change_id.shortest(8), - pad_end(8, truncate_end(8, author.username())), + pad_end(8, truncate_end(8, author.email().local())), committer.timestamp().local().format('%Y-%m-%d %H:%M:%S'), ) ''' @@ -72,7 +72,7 @@ if(root, concat( separate(" ", format_short_change_id_with_hidden_and_divergent_info(self), - if(author.email(), author.username(), email_placeholder), + if(author.email(), author.email().local(), email_placeholder), format_timestamp(committer.timestamp()), bookmarks, tags, diff --git a/cli/src/template_builder.rs b/cli/src/template_builder.rs index 13a9450d5a..d3b019700d 100644 --- a/cli/src/template_builder.rs +++ b/cli/src/template_builder.rs @@ -38,6 +38,7 @@ use crate::template_parser::UnaryOp; use crate::templater::CoalesceTemplate; use crate::templater::ConcatTemplate; use crate::templater::ConditionalTemplate; +use crate::templater::Email; use crate::templater::LabelTemplate; use crate::templater::ListPropertyTemplate; use crate::templater::ListTemplate; @@ -71,6 +72,7 @@ pub trait TemplateLanguage<'a> { property: impl TemplateProperty> + 'a, ) -> Self::Property; fn wrap_signature(property: impl TemplateProperty + 'a) -> Self::Property; + fn wrap_email(property: impl TemplateProperty + 'a) -> Self::Property; fn wrap_size_hint(property: impl TemplateProperty + 'a) -> Self::Property; fn wrap_timestamp(property: impl TemplateProperty + 'a) -> Self::Property; fn wrap_timestamp_range( @@ -117,6 +119,7 @@ macro_rules! impl_core_wrap_property_fns { wrap_integer(i64) => Integer, wrap_integer_opt(Option) => IntegerOpt, wrap_signature(jj_lib::backend::Signature) => Signature, + wrap_email($crate::templater::Email) => Email, wrap_size_hint($crate::templater::SizeHint) => SizeHint, wrap_timestamp(jj_lib::backend::Timestamp) => Timestamp, wrap_timestamp_range($crate::templater::TimestampRange) => TimestampRange, @@ -179,6 +182,7 @@ pub enum CoreTemplatePropertyKind<'a> { Integer(Box + 'a>), IntegerOpt(Box> + 'a>), Signature(Box + 'a>), + Email(Box + 'a>), SizeHint(Box + 'a>), Timestamp(Box + 'a>), TimestampRange(Box + 'a>), @@ -206,6 +210,7 @@ impl<'a> IntoTemplateProperty<'a> for CoreTemplatePropertyKind<'a> { CoreTemplatePropertyKind::Integer(_) => "Integer", CoreTemplatePropertyKind::IntegerOpt(_) => "Option", CoreTemplatePropertyKind::Signature(_) => "Signature", + CoreTemplatePropertyKind::Email(_) => "Email", CoreTemplatePropertyKind::SizeHint(_) => "SizeHint", CoreTemplatePropertyKind::Timestamp(_) => "Timestamp", CoreTemplatePropertyKind::TimestampRange(_) => "TimestampRange", @@ -228,6 +233,9 @@ impl<'a> IntoTemplateProperty<'a> for CoreTemplatePropertyKind<'a> { Some(Box::new(property.map(|opt| opt.is_some()))) } CoreTemplatePropertyKind::Signature(_) => None, + CoreTemplatePropertyKind::Email(property) => Some(Box::new( + property.map(|e| !e.local.is_empty() || e.domain.is_some()), + )), CoreTemplatePropertyKind::SizeHint(_) => None, CoreTemplatePropertyKind::Timestamp(_) => None, CoreTemplatePropertyKind::TimestampRange(_) => None, @@ -267,6 +275,7 @@ impl<'a> IntoTemplateProperty<'a> for CoreTemplatePropertyKind<'a> { CoreTemplatePropertyKind::Integer(property) => Some(property.into_template()), CoreTemplatePropertyKind::IntegerOpt(property) => Some(property.into_template()), CoreTemplatePropertyKind::Signature(property) => Some(property.into_template()), + CoreTemplatePropertyKind::Email(property) => Some(property.into_template()), CoreTemplatePropertyKind::SizeHint(_) => None, CoreTemplatePropertyKind::Timestamp(property) => Some(property.into_template()), CoreTemplatePropertyKind::TimestampRange(property) => Some(property.into_template()), @@ -292,6 +301,7 @@ impl<'a> IntoTemplateProperty<'a> for CoreTemplatePropertyKind<'a> { (CoreTemplatePropertyKind::Integer(_), _) => None, (CoreTemplatePropertyKind::IntegerOpt(_), _) => None, (CoreTemplatePropertyKind::Signature(_), _) => None, + (CoreTemplatePropertyKind::Email(_), _) => None, (CoreTemplatePropertyKind::SizeHint(_), _) => None, (CoreTemplatePropertyKind::Timestamp(_), _) => None, (CoreTemplatePropertyKind::TimestampRange(_), _) => None, @@ -314,6 +324,7 @@ impl<'a> IntoTemplateProperty<'a> for CoreTemplatePropertyKind<'a> { (CoreTemplatePropertyKind::Integer(_), _) => None, (CoreTemplatePropertyKind::IntegerOpt(_), _) => None, (CoreTemplatePropertyKind::Signature(_), _) => None, + (CoreTemplatePropertyKind::Email(_), _) => None, (CoreTemplatePropertyKind::SizeHint(_), _) => None, (CoreTemplatePropertyKind::Timestamp(_), _) => None, (CoreTemplatePropertyKind::TimestampRange(_), _) => None, @@ -360,6 +371,7 @@ pub struct CoreTemplateBuildFnTable<'a, L: TemplateLanguage<'a> + ?Sized> { pub string_methods: TemplateBuildMethodFnMap<'a, L, String>, pub boolean_methods: TemplateBuildMethodFnMap<'a, L, bool>, pub integer_methods: TemplateBuildMethodFnMap<'a, L, i64>, + pub email_methods: TemplateBuildMethodFnMap<'a, L, Email>, pub signature_methods: TemplateBuildMethodFnMap<'a, L, Signature>, pub size_hint_methods: TemplateBuildMethodFnMap<'a, L, SizeHint>, pub timestamp_methods: TemplateBuildMethodFnMap<'a, L, Timestamp>, @@ -383,6 +395,7 @@ impl<'a, L: TemplateLanguage<'a> + ?Sized> CoreTemplateBuildFnTable<'a, L> { boolean_methods: HashMap::new(), integer_methods: HashMap::new(), signature_methods: builtin_signature_methods(), + email_methods: builtin_email_methods(), size_hint_methods: builtin_size_hint_methods(), timestamp_methods: builtin_timestamp_methods(), timestamp_range_methods: builtin_timestamp_range_methods(), @@ -396,6 +409,7 @@ impl<'a, L: TemplateLanguage<'a> + ?Sized> CoreTemplateBuildFnTable<'a, L> { boolean_methods: HashMap::new(), integer_methods: HashMap::new(), signature_methods: HashMap::new(), + email_methods: HashMap::new(), size_hint_methods: HashMap::new(), timestamp_methods: HashMap::new(), timestamp_range_methods: HashMap::new(), @@ -409,6 +423,7 @@ impl<'a, L: TemplateLanguage<'a> + ?Sized> CoreTemplateBuildFnTable<'a, L> { boolean_methods, integer_methods, signature_methods, + email_methods, size_hint_methods, timestamp_methods, timestamp_range_methods, @@ -419,6 +434,7 @@ impl<'a, L: TemplateLanguage<'a> + ?Sized> CoreTemplateBuildFnTable<'a, L> { merge_fn_map(&mut self.boolean_methods, boolean_methods); merge_fn_map(&mut self.integer_methods, integer_methods); merge_fn_map(&mut self.signature_methods, signature_methods); + merge_fn_map(&mut self.email_methods, email_methods); merge_fn_map(&mut self.size_hint_methods, size_hint_methods); merge_fn_map(&mut self.timestamp_methods, timestamp_methods); merge_fn_map(&mut self.timestamp_range_methods, timestamp_range_methods); @@ -493,6 +509,11 @@ impl<'a, L: TemplateLanguage<'a> + ?Sized> CoreTemplateBuildFnTable<'a, L> { let build = template_parser::lookup_method(type_name, table, function)?; build(language, diagnostics, build_ctx, property, function) } + CoreTemplatePropertyKind::Email(property) => { + let table = &self.email_methods; + let build = template_parser::lookup_method(type_name, table, function)?; + build(language, diagnostics, build_ctx, property, function) + } CoreTemplatePropertyKind::SizeHint(property) => { let table = &self.size_hint_methods; let build = template_parser::lookup_method(type_name, table, function)?; @@ -871,14 +892,19 @@ fn builtin_signature_methods<'a, L: TemplateLanguage<'a> + ?Sized>( "email", |_language, _diagnostics, _build_ctx, self_property, function| { function.expect_no_arguments()?; - let out_property = self_property.map(|signature| signature.email); - Ok(L::wrap_string(out_property)) + let out_property = self_property.map(|signature| signature.email.into()); + Ok(L::wrap_email(out_property)) }, ); map.insert( "username", - |_language, _diagnostics, _build_ctx, self_property, function| { + |_language, diagnostics, _build_ctx, self_property, function| { function.expect_no_arguments()?; + // TODO: Remove in jj 0.30+ + diagnostics.add_warning(TemplateParseError::expression( + "username() is deprecated; use email().local() instead", + function.name_span, + )); let out_property = self_property.map(|signature| { let (username, _) = text_util::split_email(&signature.email); username.to_owned() @@ -897,6 +923,30 @@ fn builtin_signature_methods<'a, L: TemplateLanguage<'a> + ?Sized>( map } +fn builtin_email_methods<'a, L: TemplateLanguage<'a> + ?Sized>( +) -> TemplateBuildMethodFnMap<'a, L, Email> { + // Not using maplit::hashmap!{} or custom declarative macro here because + // code completion inside macro is quite restricted. + let mut map = TemplateBuildMethodFnMap::::new(); + map.insert( + "local", + |_language, _diagnostics, _build_ctx, self_property, function| { + function.expect_no_arguments()?; + let out_property = self_property.map(|email| email.local); + Ok(L::wrap_string(out_property)) + }, + ); + map.insert( + "domain", + |_language, _diagnostics, _build_ctx, self_property, function| { + function.expect_no_arguments()?; + let out_property = self_property.map(|email| email.domain.unwrap_or_default()); + Ok(L::wrap_string(out_property)) + }, + ); + map +} + fn builtin_size_hint_methods<'a, L: TemplateLanguage<'a> + ?Sized>( ) -> TemplateBuildMethodFnMap<'a, L, SizeHint> { // Not using maplit::hashmap!{} or custom declarative macro here because diff --git a/cli/src/templater.rs b/cli/src/templater.rs index 05f7db4032..5fef355939 100644 --- a/cli/src/templater.rs +++ b/cli/src/templater.rs @@ -27,6 +27,7 @@ use crate::formatter::FormatRecorder; use crate::formatter::Formatter; use crate::formatter::LabeledWriter; use crate::formatter::PlainTextFormatter; +use crate::text_util; use crate::time_util; /// Represents printable type or compiled template containing placeholder value. @@ -75,13 +76,41 @@ impl Template for Signature { } if !self.email.is_empty() { write!(formatter, "<")?; - write!(formatter.labeled("email"), "{}", self.email)?; + let email: Email = self.email.clone().into(); + email.format(formatter)?; write!(formatter, ">")?; } Ok(()) } } +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Email { + pub local: String, + pub domain: Option, +} + +impl From for Email { + fn from(value: String) -> Self { + let (local, domain) = text_util::split_email(&value); + Email { + local: local.to_owned(), + domain: domain.map(|d| d.to_owned()), + } + } +} + +impl Template for Email { + fn format(&self, formatter: &mut TemplateFormatter) -> io::Result<()> { + write!(formatter.labeled("local"), "{}", self.local)?; + if let Some(domain) = &self.domain { + write!(formatter, "@")?; + write!(formatter.labeled("domain"), "{domain}")?; + } + Ok(()) + } +} + // In template language, an integer value is represented as i64. However, we use // usize here because it's more convenient to guarantee that the lower value is // bounded to 0. diff --git a/cli/tests/test_commit_template.rs b/cli/tests/test_commit_template.rs index 5958dd795a..c875d0cd6c 100644 --- a/cli/tests/test_commit_template.rs +++ b/cli/tests/test_commit_template.rs @@ -431,14 +431,14 @@ fn test_log_builtin_templates_colored_debug() { insta::assert_snapshot!(render(r#"builtin_log_oneline"#), @r#" <> <><><><><><><><><><><><><><><><> - <> <><><><><><><><><><><><><><> + <> <><><><><><><><><><><><><><> <> <><><><><><><><> "#); insta::assert_snapshot!(render(r#"builtin_log_compact"#), @r#" <> <><><><><><><><><><><><> │ <><><><> - <> <><><><><><><><><><> + <> <><><><><><><><><><><><> │ <><><><> <> <><><><><><><><> "#); @@ -447,7 +447,7 @@ fn test_log_builtin_templates_colored_debug() { <> <><><><><><><><><><><><> │ <><><><> │ <> - <> <><><><><><><><><><> + <> <><><><><><><><><><><><> │ <><><><> │ <> <> <><><><><><><><> @@ -465,8 +465,8 @@ fn test_log_builtin_templates_colored_debug() { │ <> <> <><><> │ <><><> - │ <><><><>< (>><><> - │ <><><><>< (>><><> + │ <><><><><><>< (>><><> + │ <><><><><><>< (>><><> │ <> │ <><> │ <> diff --git a/cli/tests/test_log_command.rs b/cli/tests/test_log_command.rs index 968139e287..4e4b946fbd 100644 --- a/cli/tests/test_log_command.rs +++ b/cli/tests/test_log_command.rs @@ -741,7 +741,7 @@ fn test_log_author_format() { &repo_path, &[ "--config-toml", - &format!("{decl}='signature.username()'"), + &format!("{decl}='signature.email().local()'"), "log", "--revisions=@", ], diff --git a/cli/tests/test_show_command.rs b/cli/tests/test_show_command.rs index 01ac6d350b..2d18dc360a 100644 --- a/cli/tests/test_show_command.rs +++ b/cli/tests/test_show_command.rs @@ -83,8 +83,8 @@ fn test_show_basic() { insta::assert_snapshot!(stdout, @r#" Commit ID: <> Change ID: <> - Author : <> <<>> (<>) - Committer: <> <<>> (<>) + Author : <> <<><><>> (<>) + Committer: <> <<><><>> (<>) <> @@ -170,8 +170,8 @@ fn test_show_basic() { insta::assert_snapshot!(stdout, @r#" Commit ID: <> Change ID: <> - Author : <> <<>> (<>) - Committer: <> <<>> (<>) + Author : <> <<><><>> (<>) + Committer: <> <<><><>> (<>) <> diff --git a/cli/tests/test_templater.rs b/cli/tests/test_templater.rs index 7c62a57157..5a7c8e37ad 100644 --- a/cli/tests/test_templater.rs +++ b/cli/tests/test_templater.rs @@ -129,11 +129,12 @@ fn test_template_parse_warning() { local_branches, remote_branches, self.contained_in('branches()'), + author.username(), ) "#}; let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-r@", "-T", template]); insta::assert_snapshot!(stdout, @r#" - @ false + @ false test.user │ ~ "#); @@ -172,6 +173,13 @@ fn test_template_parse_warning() { | ^------^ | = branches() is deprecated; use bookmarks() instead + Warning: In template expression + --> 6:10 + | + 6 | author.username(), + | ^------^ + | + = username() is deprecated; use email().local() instead "#); } diff --git a/docs/templates.md b/docs/templates.md index 30bb71349b..67691ce7c1 100644 --- a/docs/templates.md +++ b/docs/templates.md @@ -129,6 +129,13 @@ The following methods are defined. * `.short([len: Integer]) -> String` * `.shortest([min_len: Integer]) -> ShortestIdPrefix`: Shortest unique prefix. +### Email type + +The following methods are defined. + +* `.local() -> String` +* `.domain() -> String` + ### Integer type No methods are defined. @@ -212,8 +219,7 @@ The following methods are defined. The following methods are defined. * `.name() -> String` -* `.email() -> String` -* `.username() -> String` +* `.email() -> Email` * `.timestamp() -> Timestamp` ### SizeHint type