Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

templater: add Email template type, deprecate Signature.username() #5079

Merged
merged 1 commit into from
Dec 15, 2024
+148 −19
Merged

templater: add Email template type, deprecate Signature.username() #5079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,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
Expand All @@ -44,6 +47,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.
Expand Down
4 changes: 2 additions & 2 deletions cli/src/config/templates.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
)
'''
Expand Down Expand Up @@ -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,
Expand Down
92 changes: 89 additions & 3 deletions cli/src/template_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -71,6 +72,7 @@ pub trait TemplateLanguage<'a> {
property: impl TemplateProperty<Output = Option<i64>> + 'a,
) -> Self::Property;
fn wrap_signature(property: impl TemplateProperty<Output = Signature> + 'a) -> Self::Property;
fn wrap_email(property: impl TemplateProperty<Output = Email> + 'a) -> Self::Property;
fn wrap_size_hint(property: impl TemplateProperty<Output = SizeHint> + 'a) -> Self::Property;
fn wrap_timestamp(property: impl TemplateProperty<Output = Timestamp> + 'a) -> Self::Property;
fn wrap_timestamp_range(
Expand Down Expand Up @@ -117,6 +119,7 @@ macro_rules! impl_core_wrap_property_fns {
wrap_integer(i64) => Integer,
wrap_integer_opt(Option<i64>) => 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,
Expand Down Expand Up @@ -179,6 +182,7 @@ pub enum CoreTemplatePropertyKind<'a> {
Integer(Box<dyn TemplateProperty<Output = i64> + 'a>),
IntegerOpt(Box<dyn TemplateProperty<Output = Option<i64>> + 'a>),
Signature(Box<dyn TemplateProperty<Output = Signature> + 'a>),
Email(Box<dyn TemplateProperty<Output = Email> + 'a>),
SizeHint(Box<dyn TemplateProperty<Output = SizeHint> + 'a>),
Timestamp(Box<dyn TemplateProperty<Output = Timestamp> + 'a>),
TimestampRange(Box<dyn TemplateProperty<Output = TimestampRange> + 'a>),
Expand Down Expand Up @@ -206,6 +210,7 @@ impl<'a> IntoTemplateProperty<'a> for CoreTemplatePropertyKind<'a> {
CoreTemplatePropertyKind::Integer(_) => "Integer",
CoreTemplatePropertyKind::IntegerOpt(_) => "Option<Integer>",
CoreTemplatePropertyKind::Signature(_) => "Signature",
CoreTemplatePropertyKind::Email(_) => "Email",
CoreTemplatePropertyKind::SizeHint(_) => "SizeHint",
CoreTemplatePropertyKind::Timestamp(_) => "Timestamp",
CoreTemplatePropertyKind::TimestampRange(_) => "TimestampRange",
Expand All @@ -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.0.is_empty())))
}
CoreTemplatePropertyKind::SizeHint(_) => None,
CoreTemplatePropertyKind::Timestamp(_) => None,
CoreTemplatePropertyKind::TimestampRange(_) => None,
Expand Down Expand Up @@ -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()),
Expand All @@ -280,18 +289,28 @@ impl<'a> IntoTemplateProperty<'a> for CoreTemplatePropertyKind<'a> {
(CoreTemplatePropertyKind::String(lhs), CoreTemplatePropertyKind::String(rhs)) => {
Some(Box::new((lhs, rhs).map(|(l, r)| l == r)))
}
(CoreTemplatePropertyKind::String(lhs), CoreTemplatePropertyKind::Email(rhs)) => {
Some(Box::new((lhs, rhs).map(|(l, r)| l == r.0)))
}
(CoreTemplatePropertyKind::Boolean(lhs), CoreTemplatePropertyKind::Boolean(rhs)) => {
Some(Box::new((lhs, rhs).map(|(l, r)| l == r)))
}
(CoreTemplatePropertyKind::Integer(lhs), CoreTemplatePropertyKind::Integer(rhs)) => {
Some(Box::new((lhs, rhs).map(|(l, r)| l == r)))
}
(CoreTemplatePropertyKind::Email(lhs), CoreTemplatePropertyKind::Email(rhs)) => {
Some(Box::new((lhs, rhs).map(|(l, r)| l == r)))
}
(CoreTemplatePropertyKind::Email(lhs), CoreTemplatePropertyKind::String(rhs)) => {
Some(Box::new((lhs, rhs).map(|(l, r)| l.0 == r)))
}
bnjmnt4n marked this conversation as resolved.
Show resolved Hide resolved
(CoreTemplatePropertyKind::String(_), _) => None,
(CoreTemplatePropertyKind::StringList(_), _) => None,
(CoreTemplatePropertyKind::Boolean(_), _) => None,
(CoreTemplatePropertyKind::Integer(_), _) => None,
(CoreTemplatePropertyKind::IntegerOpt(_), _) => None,
(CoreTemplatePropertyKind::Signature(_), _) => None,
(CoreTemplatePropertyKind::Email(_), _) => None,
(CoreTemplatePropertyKind::SizeHint(_), _) => None,
(CoreTemplatePropertyKind::Timestamp(_), _) => None,
(CoreTemplatePropertyKind::TimestampRange(_), _) => None,
Expand All @@ -314,6 +333,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,
Expand Down Expand Up @@ -360,6 +380,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>,
Expand All @@ -383,6 +404,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(),
Expand All @@ -396,6 +418,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(),
Expand All @@ -409,6 +432,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,
Expand All @@ -419,6 +443,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);
Expand Down Expand Up @@ -493,6 +518,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)?;
Expand Down Expand Up @@ -871,14 +901,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()
Expand All @@ -897,6 +932,36 @@ 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::<L, Email>::new();
map.insert(
"local",
|_language, _diagnostics, _build_ctx, self_property, function| {
function.expect_no_arguments()?;
let out_property = self_property.map(|email| {
let (local, _) = text_util::split_email(&email.0);
local.to_owned()
});
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| {
let (_, domain) = text_util::split_email(&email.0);
domain.unwrap_or_default().to_owned()
});
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
Expand Down Expand Up @@ -2084,6 +2149,15 @@ mod tests {
|
= Expected expression of type "Boolean", but actual type is "ListTemplate"
"###);

env.add_keyword("empty_email", || {
L::wrap_email(Literal(Email("".to_owned())))
});
env.add_keyword("nonempty_email", || {
L::wrap_email(Literal(Email("local@domain".to_owned())))
});
insta::assert_snapshot!(env.render_ok(r#"if(empty_email, true, false)"#), @"false");
insta::assert_snapshot!(env.render_ok(r#"if(nonempty_email, true, false)"#), @"true");
}

#[test]
Expand Down Expand Up @@ -2125,6 +2199,12 @@ mod tests {
#[test]
fn test_logical_operation() {
let mut env = TestTemplateEnv::new();
env.add_keyword("email1", || {
L::wrap_email(Literal(Email("local-1@domain".to_owned())))
});
env.add_keyword("email2", || {
L::wrap_email(Literal(Email("local-2@domain".to_owned())))
});

insta::assert_snapshot!(env.render_ok(r#"!false"#), @"true");
insta::assert_snapshot!(env.render_ok(r#"false || !false"#), @"true");
Expand All @@ -2141,6 +2221,12 @@ mod tests {
insta::assert_snapshot!(env.render_ok(r#"'a' == 'b'"#), @"false");
insta::assert_snapshot!(env.render_ok(r#"'a' != 'a'"#), @"false");
insta::assert_snapshot!(env.render_ok(r#"'a' != 'b'"#), @"true");
insta::assert_snapshot!(env.render_ok(r#"email1 == email1"#), @"true");
insta::assert_snapshot!(env.render_ok(r#"email1 == email2"#), @"false");
insta::assert_snapshot!(env.render_ok(r#"email1 == 'local-1@domain'"#), @"true");
insta::assert_snapshot!(env.render_ok(r#"email1 != 'local-2@domain'"#), @"true");
bnjmnt4n marked this conversation as resolved.
Show resolved Hide resolved
insta::assert_snapshot!(env.render_ok(r#"'local-1@domain' == email1"#), @"true");
insta::assert_snapshot!(env.render_ok(r#"'local-2@domain' != email1"#), @"true");

insta::assert_snapshot!(env.render_ok(r#" !"" "#), @"true");
insta::assert_snapshot!(env.render_ok(r#" "" || "a".lines() "#), @"true");
Expand Down
25 changes: 24 additions & 1 deletion cli/src/templater.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -75,13 +76,35 @@ 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 String);

impl From<String> for Email {
fn from(value: String) -> Self {
Self(value)
}
}

impl Template for Email {
fn format(&self, formatter: &mut TemplateFormatter) -> io::Result<()> {
let (local, domain) = text_util::split_email(&self.0);
write!(formatter.labeled("local"), "{local}")?;
if let Some(domain) = 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.
Expand Down
Loading
Loading