-
Notifications
You must be signed in to change notification settings - Fork 157
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
Add meta robots tags helper #82
Add meta robots tags helper #82
Conversation
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.
I don't think it's accurate to have two string fields, because not all strings are valid <meta robots>
tags.
I think a better interface would be
struct MetaRobots {
follow: bool,
index: bool,
}
which the Render
impl expands into the full tag.
maud_extras/lib.rs
Outdated
impl<T: AsRef<str>, U: AsRef<str>> Render for MetaRobots<T, U> { | ||
fn render(&self) -> Markup { | ||
html! { | ||
meta name="robots" content=(format!("{}, {}", self.0.as_ref(), self.1.as_ref())) / |
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.
Suggest writing this as
meta name="robots" content={
(self.0.as_ref())
", "
(self.1.as_ref())
} /
since this avoids using the std::fmt
machinery.
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.
indeed, thanks for the tip !
cd7529e
to
93dc4bf
Compare
93dc4bf
to
3269d7b
Compare
@lfairy hey sorry for forgetting this 1, updated according to your suggestions, for me usage now is a little cumbersome, what do you think about functoin new(index: bool, follow: bool) -> MetaRobots ? :) |
4c77398
to
2ffe69b
Compare
I think your rebase got messed up. You may just want to reset and do a fresh commit on master. |
2ffe69b
to
0a28232
Compare
you are right, did soft reset and new commit |
maud_extras/lib.rs
Outdated
let index = if self.index { "index" } else { "noindex" }; | ||
let follow = if self.follow { "follow" } else { "nofollow" }; | ||
html! { | ||
meta name="robots" content=(format!("{},{}", index, follow)) / |
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.
This is more efficient:
content={ (index) "," (follow) }
@ernestas-poskus I'm okay with adding a |
0a28232
to
fe4413a
Compare
fixed. tried new let markup = html! { (MetaRobots::new(true,false)) }; looks odd and not self explanatory left it as is |
fe4413a
to
86050c8
Compare
Looks good, thanks! |
Adding meta robots tags helper, I can reuse MetaProperty if needed.