-
Notifications
You must be signed in to change notification settings - Fork 330
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.
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
base: main
Are you sure you want to change the base?
Conversation
The `Signature.email()` method is also updated to return the new Email type. The `Signature.username()` method is deprecated for `Signature.email().local()`.
aa7ae95
to
eee3065
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
@@ -217,6 +222,9 @@ impl<'a> IntoTemplateProperty<'a> for CoreTemplatePropertyKind<'a> { | |||
CoreTemplatePropertyKind::StringList(property) => { | |||
Some(Box::new(property.map(|l| !l.is_empty()))) | |||
} | |||
CoreTemplatePropertyKind::Email(property) => Some(Box::new( | |||
property.map(|e| !e.local.is_empty() || e.domain.is_some()), | |||
)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: sort in definition order?
@@ -820,14 +840,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.clone().into()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: .clone()
isn't needed
pub struct Email { | ||
pub local: String, | ||
pub domain: Option<String>, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: might be simpler to implement as a new type Email(pub String)
(because the type implements implicit cast to boolean based on string)
* `.timestamp() -> Timestamp` | ||
|
||
### Email type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: types are sorted alphabetically in doc
The
Signature.email()
method is also updated to return the new Email type. TheSignature.username()
method is deprecated forSignature.email().local()
.This is an alternative to #4687.
Checklist
If applicable:
CHANGELOG.md