From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 71A1B9AD34 for ; Mon, 16 Oct 2023 13:30:28 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4581416D4A for ; Mon, 16 Oct 2023 13:29:58 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 16 Oct 2023 13:29:57 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id C671140FD4 for ; Mon, 16 Oct 2023 13:29:56 +0200 (CEST) Message-ID: Date: Mon, 16 Oct 2023 13:29:55 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.14.0 To: pve-devel@lists.proxmox.com References: <20231012130208.181749-1-f.schauer@proxmox.com> Content-Language: en-US From: Filip Schauer In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 1.107 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_ASCII_DIVIDERS 0.8 Email that uses ascii formatting dividers and possible spam tricks KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -3.339 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH v2 installer] fix #4869: Make management interface selection more verbose X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Oct 2023 11:30:28 -0000 On 13/10/2023 12:36, Christoph Heiss wrote: > See inline comments. Also, `cargo fmt` please :^) > > On Thu, Oct 12, 2023 at 03:02:08PM +0200, Filip Schauer wrote: >> [..] >> diff --git a/proxinstall b/proxinstall >> index d5b2565..51170cd 100755 >> --- a/proxinstall >> +++ b/proxinstall >> @@ -347,7 +347,9 @@ sub create_ipconf_view { >> >> my $get_device_desc = sub { >> my $iface = shift; >> - return "$iface->{name} - $iface->{mac} ($iface->{driver})"; >> + my $iface_state_symbol = "\N{U+25EF}"; >> + $iface_state_symbol = "\N{U+1F7E2}" if ($iface->{state} eq "UP"); >> + return "$iface_state_symbol $iface->{name} - $iface->{mac} ($iface->{driver})"; > ^ Here we have e.g. " eno1 - 12:34:56:78:9a:bc (r8169)". > >> }; >> >> my $run_env = Proxmox::Install::RunEnv::get(); >> diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs >> index 3f01713..7e46f33 100644 >> --- a/proxmox-tui-installer/src/main.rs >> +++ b/proxmox-tui-installer/src/main.rs >> @@ -548,20 +548,28 @@ fn password_dialog(siv: &mut Cursive) -> InstallerView { >> fn network_dialog(siv: &mut Cursive) -> InstallerView { >> let state = siv.user_data::().unwrap(); >> let options = &state.options.network; >> - let ifnames = state.runtime_info.network.interfaces.keys(); >> + >> + let mut management_interface_select_view = SelectView::new().popup(); >> + for (key, value) in state.runtime_info.network.interfaces.iter() { >> + let interface_state_symbol = if value.state == "UP" { >> + "\x1b[1;92m\u{25CF}" >> + } else { >> + "\x1b[1;31m\u{25CF}" >> + }; > One thing I noticed while testing the TUI installer is the > setting of the color here fsck up the colors of the other entries. > Colors/effects applied using ANSI escape sequences do not reset > automatically, so this must be done after the unicode symbol manually > using "\x1b[1;30m". > > E.g. with multiple NICS, nics in UP are sometimes displayed entirely in > green, in non-UP entirely red. And scrolling through the list changes > that completely. > > [ To properly display colors in Cursive, you'd normally use a > `StyledString`, but - after quickly trying it out - seems broken for > SelectView (not sure why, I might investigate that some time). ] > > So although a bit hacky, seems to work here fine (still seems a bit > broken under VGA for me, but that is just some random artifact it seems, > as 30 is the correct color code in any case). > >> + >> + let interface_label = format!("{0} - {1} {2}", value.name, value.mac, interface_state_symbol); > ^ And here we have "eno1 - 12:34:56:78:9a:bc ". > > Just for the sake of consistency they should be the same between GUI and > TUI. > I would propose " - ", and just drop the driver part > completely in the GUI. The latter does not provide any real value IMO, > at least not I could come up with any. In the TUI installer, placing the symbol at the front changes the color of the remaining text that comes after it. Resetting the color with \x1b[1;30m is also not an option since it sets the text color to gray, which makes it inconsistent with the selectable elements and hard to read. >> + management_interface_select_view.add_item(interface_label, key.to_string()); >> + } >> >> let inner = FormView::new() >> .child( >> "Management interface", >> - SelectView::new() >> - .popup() >> - .with_all_str(ifnames.clone()) >> - .selected( >> - ifnames >> - .clone() >> - .position(|ifname| ifname == &options.ifname) >> - .unwrap_or_default(), >> - ), >> + management_interface_select_view.selected( >> + state.runtime_info.network.interfaces.keys() >> + .clone() >> + .position(|ifname| ifname == &options.ifname) >> + .unwrap_or_default(), >> + ), >> ) >> .child( >> "Hostname (FQDN)", >> diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs >> index 942e319..dbff3da 100644 >> --- a/proxmox-tui-installer/src/setup.rs >> +++ b/proxmox-tui-installer/src/setup.rs >> @@ -443,6 +443,8 @@ pub struct Interface { >> >> pub index: usize, >> >> + pub state: String, > This can be an enum for cleanliness, as this field is well defined. All > possible states as recognized (and thus can be put out) by iproute2 are > defined here: > https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/ip/ipaddress.c?h=v6.5.0#n119 > >> + >> pub mac: String, >> >> #[serde(default)] >> -- >> 2.39.2 >> >> >> >> _______________________________________________ >> pve-devel mailing list >> pve-devel@lists.proxmox.com >> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >> >> > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > >