fix(rustup): installed toolchains for legacy rustup #1683
Conversation
reneleonhardt
force-pushed
the
fix/installed-toolchains
branch
2 times, most recently
from
066ab7c to
1ea2fd5
Compare
src/rustup.rs
Outdated
| // Emulate --quiet by removing suffixes like " (active, default)" or " (override)" manually | ||
| Ok(out | ||
| .lines() | ||
| .map(|l| SUFFIX_REGEX.replace(l.trim(), "").into_owned()) |
There was a problem hiding this comment.
Regex is not needed for this.
There was a problem hiding this comment.
.map(|l| l.split_once(" (").map(|(a, _)| a).unwrap_or(l))
There was a problem hiding this comment.
or what you did with split, I just have a habit of using split_once when I really only need one split :D
There was a problem hiding this comment.
Let me know if it can be improved further.
Can you squash yourself upon merge or should I do it manually?
There was a problem hiding this comment.
Please squash the commits and we can merge. If possible I'd like a test for this also, but no stress we can fo that later
There was a problem hiding this comment.
I couldn't find any tests for rustup.rs and I know that you don't like new crates like visibility to allow doctests...
#1684
reneleonhardt
force-pushed
the
fix/installed-toolchains
branch
2 times, most recently
from
bf62f44 to
db297cb
Compare
reneleonhardt
force-pushed
the
fix/installed-toolchains
branch
from
db297cb to
3e09576
Compare
Fixes #1678
Unfortunately some installations use legacy versions, my previous fix was only working for the current rustup >= 1.28.0.