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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bnjmnt4n
Copy link
Collaborator

The Signature.email() method is also updated to return the new Email type. The Signature.username() method is deprecated for Signature.email().local().

This is an alternative to #4687.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

The `Signature.email()` method is also updated to return the new Email
type. The `Signature.username()` method is deprecated for
`Signature.email().local()`.
@bnjmnt4n bnjmnt4n force-pushed the bnjmnt4n/push-knznzkuuumll branch from aa7ae95 to eee3065 Compare December 12, 2024 03:47
Copy link
Collaborator

@yuja yuja left a 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()),
)),
Copy link
Collaborator

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());
Copy link
Collaborator

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

Comment on lines +88 to +91
pub struct Email {
pub local: String,
pub domain: Option<String>,
}
Copy link
Collaborator

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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants