Re: [pve-devel] [PATCH proxmox v3 2/2] network-types: add hostname type

[Stefan Hanreich <s.hanreich@proxmox.com>]

On 4/4/25 09:51, Stefan Hanreich wrote:
>>> +///
>>> +/// It checks for the following conditions:
>>> +/// * At most 63 characters long.
>>> +/// * It must not start or end with a hyphen.
>>> +/// * Must only contain ASCII alphanumeric characters as well as hyphens.
>>> +/// * It must not be purely numerical.
>>> +#[derive(Debug, Deserialize, Serialize, Clone, Eq, Hash, PartialOrd, Ord, PartialEq)]
>>> +pub struct Hostname(String);
>>> +
>>> +impl std::str::FromStr for Hostname {
>>> +    type Err = HostnameError;
>>> +
>>> +    fn from_str(hostname: &str) -> Result<Self, Self::Err> {
>>> +        Self::new(hostname)
>>> +    }
>>> +}
>>> +
>>> +impl AsRef<str> for Hostname {
>>> +    fn as_ref(&self) -> &str {
>>> +        &self.0
>>> +    }
>>> +}
>>> +
>>> +impl Display for Hostname {
>>> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
>>> +        self.0.fmt(f)
>>> +    }
>>> +}
>>> +
>>> +impl Hostname {
>>> +    /// Constructs a new hostname from a string
>>> +    ///
>>> +    /// This function accepts characters in any case, but the resulting hostname will be
>>> +    /// lowercased.
>>> +    pub fn new(name_ref: impl AsRef<str>) -> Result<Self, HostnameError> {
>>
>> Nit: I'd recommend using a `check()` function which does not create the
>> `Hostname` itself, because then:
>>
>> - in `FromStr` we know we have a reference (&str) and need to clone.
>> - We could add a `TryFrom<&str>` wich just uses `.parse()`
>> - We could add a `TryFrom<String>` which avoids the clone.

What about a constructor that just takes String (if a function needs to
own the value it should demand a String anyway) and then calling that
constructor from the proposed trait implementations? Maybe something
more generic than String as a param, but that could usually be tacked on
later without breaking the API.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel