Strip angle brackets from author email before passing to template. #6243
Conversation
|
I'm only a few days into learning Rust, so please let me know if I am doing something totally off-base here. |
btashton
force-pushed
the
strip_email_bracket
branch
from
cccb9d8
to
8de22b0
Compare
src/cargo/ops/cargo_new.rs
Outdated
| @@ -660,7 +660,15 @@ fn discover_author() -> CargoResult<(String, Option<String>)> { | |||
| .or_else(|| get_environment_variable(&email_variables[3..])); | |||
|
|
|||
| let name = name.trim().to_string(); | |||
| let email = email.map(|s| s.trim().to_string()); | |||
| let email = email.map(|mut s| { | |||
| s = s.trim().to_string(); | |||
There was a problem hiding this comment.
This might be best done with a bit more slicing, like:
let mut s = s.trim();
if s.starts_with("<") && s.ends_with(">") {
s = &s[1..s.len() - 1];
}
s.to_string()There was a problem hiding this comment.
Also, could a comment be added why it's being trimmed?
There was a problem hiding this comment.
I'll fix it up with the slicing. But I actually do not know why there is a trim in there, it is from the original code. My guess is that some sources might include a newline and we want to make sure that is gone.
There was a problem hiding this comment.
@alexcrichton PR has been updated with this change minus the comment.
There was a problem hiding this comment.
Ah yeah the trim() is sort of just for good measure, I'm not sure if it's solidly motivated one way or the other.
As a mild stylistic note as well, the { after the if clause typically comes on the same line instead of the next line
There was a problem hiding this comment.
Falling into my C habit. Fixed.
There was a problem hiding this comment.
Ok thinks! And for a comment FWIW I was just thinking of your use case of stripping the angle brackets. Would you be ok adding that?
btashton
force-pushed
the
strip_email_bracket
branch
2 times, most recently
from
1a1705a
to
5c7e8eb
Compare
Some people already have angle brackets around their email in git settings or other author sources. Right now if you create a new project the Cargo.toml would render something like: authors = ["bar <<foo@baz>>"] instead of authors = ["bar <foo@baz>"] This detects the emails that start and end with <> and removes them.
btashton
force-pushed
the
strip_email_bracket
branch
from
5c7e8eb
to
7d57948
Compare
|
@bors: r+ |
|
📌 Commit 7d57948 has been approved by |
Strip angle brackets from author email before passing to template. Some people already have angle brackets around their email in git settings or other author sources. Right now if you create a new project the Cargo.toml would render something like: `authors = ["bar <<foo@baz>>"]` instead of `authors = ["bar <foo@baz>"]` This detects the emails that start and end with `<>` and removes them.
|
☀️ Test successful - status-appveyor, status-travis |
Some people already have angle brackets around their email in
git settings or other author sources. Right now if you create
a new project the Cargo.toml would render something like:
authors = ["bar <<foo@baz>>"]instead of
authors = ["bar <foo@baz>"]This detects the emails that start and end with
<>and removes them.