Re: [pve-devel] [PATCH pve-installer v3 6/7] tui: change get_value return type for easier error handling

["Christoph Heiss" <c.heiss@proxmox.com>]

On Thu Jun 26, 2025 at 5:11 PM CEST, Michael Köppl wrote:
[..]
> diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
> index 4364489..3211c93 100644
> --- a/proxmox-tui-installer/src/views/mod.rs
> +++ b/proxmox-tui-installer/src/views/mod.rs
> @@ -387,8 +387,8 @@ where
>      }
>  }
>
> -impl FormViewGetValue<CidrAddress> for CidrAddressEditView {
> -    fn get_value(&self) -> Option<CidrAddress> {
> +impl FormViewGetValue<(IpAddr, usize)> for CidrAddressEditView {
> +    fn get_value(&self) -> Option<(IpAddr, usize)> {

I see _why_ you changed it, although it's a bit backwards - since it
makes sense that `CidrAddressEditView` would produce a `CidrAddress`.

The proper solution here is to change `FormViewGetValue::get_value()` to
return a `Result<R>` instead of an `Option<R>`, such that errors can
easily be bubbled up.
IIRC I started a series locally to do exactly that and address these
cases, but it's a bigger refactoring.

But for now, IMHO this would be okay as a stop-gap solution, instead of
holding up this series on a somewhat unrelated refactoring.

Can you please at least add a big FIXME here, that this implementation
should use `CidrAddress` (again) instead of a (IP, mask) tuple?

>          self.get_values()
>      }
>  }
> @@ -569,7 +569,7 @@ impl CidrAddressEditView {
>              .fixed_width(4)
>      }
>
> -    fn get_values(&self) -> Option<CidrAddress> {
> +    fn get_values(&self) -> Option<(IpAddr, usize)> {
>          let addr = self
>              .view
>              .get_child(0)?
> @@ -587,7 +587,7 @@ impl CidrAddressEditView {
>              .get_content()
>              .ok()?;
>
> -        CidrAddress::new(addr, mask).ok()
> +        Some((addr, mask))
>      }
>  }
>



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