Closed InfyniteHeap closed 2 weeks ago
Here you go: #3184
I don't plan to publish another crate release until we're confident this is in good shape for the foreseeable future.
@kennykerr https://github.com/rust-lang/rustup/pull/3896 looks great with the Cargo patch that points to the latest dev version of the lib! Thanks to you and @InfyniteHeap's efforts, the migration seems quite straightforward now.
I'd like to check some more things before the release, we're left with some unsafe code in rustup that might be nice to migrate into this crate.
@rami3l - glad to hear things are going well!
@djc - sounds good - please open a new issue for any bug/feature requests.
I'd like to check some more things before the release, we're left with some unsafe code in rustup that might be nice to migrate into this crate.
The reason why I add unsafe code is that, since I cannot manually construct a Value
to compare to what I got from get_value()
, I have to manually seperately convert the got Value
into Type
and &[u8]
to fit the requirements. See my latest commit and this issue.
That issue isn't clear on what it's asking for. By the way, #3190 provides a few small improvements for convertibility.
That issue isn't clear on what it's asking for. By the way, #3190 provides a few small improvements for convertibility.
My initial idea is that, I probably need a pair of generic-feasible traits that can make Value
and other types be inter-converted, as what did in the previous rustup code.
The original effect I wanted to reach out looks like this (for illustration purposes only, because this code definitely cannot be compiled):
pub fn set_value<T: TryFrom<Value>>(&self, new: Option<&T>) -> Result<()> {
self.set(new.map(|v| &Value::try_from(v).unwrap()))
}
fn set(&self, new: Option<&Value>) -> Result<()> {
let sub_key = CURRENT_USER.create(self.sub_key)?;
match new {
Some(new) => Ok(sub_key.set_value(self.value_name, new)?),
None => Ok(sub_key.remove_value(self.value_name)?),
}
}
...But now I think it is no need indeed to require such this feature.
In any cases, this question is now resolved! xd
My initial idea is that, I probably need a pair of generic-feasible traits that can make Value and other types be inter-converted, as what did in the previous rustup code.
TryFrom<Value>
and TryInto<Value>
are a pair of generic traits that can convert to/from Value
.
Your code example will work, just with TryInto
instead of TryFrom
:
pub fn set_value<T: TryInto<Value>>(&self, new: Option<T>) -> Result<()> {
self.set(new.map(|v| &v.try_into().unwrap()))
}
or alternatively (less idiomatic):
pub fn set_value<T>(&self, new: Option<T>) -> Result<()> where Value: TryFrom<T> {
self.set(new.map(|v| &Value::try_from(v).unwrap()))
}
Suggestion
Coming from https://github.com/rust-lang/rustup/pull/3896/files#r1666173552. As what we discussed in this review, we should set string values with
REG_EXPAND_SZ
type when modifyingPATH
. So, we need a way to set the string type when callingset_string()
andset_hstring()
.Appendix: We also need a way to verify whether
String
orHSTRING
hasREG_SZ
orREG_EXPAND_SZ
. See https://github.com/rust-lang/rustup/blob/044083c9cc58b0ae23eeb1a390ca17ddc6f859d2/src/cli/self_update/windows.rs#L950.