From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 6FDC21FF17E for ; Thu, 16 Oct 2025 14:02:01 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6505211F7D; Thu, 16 Oct 2025 14:02:20 +0200 (CEST) Mime-Version: 1.0 Date: Thu, 16 Oct 2025 14:01:46 +0200 Message-Id: From: "Christoph Heiss" To: "Maximiliano Sandoval" X-Mailer: aerc 0.21.0 References: <20251014132207.1171073-1-c.heiss@proxmox.com> <20251014132207.1171073-15-c.heiss@proxmox.com> In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1760616103575 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.041 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_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment 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 installer 14/14] gui: add support for pinning network interface names 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: , Reply-To: Proxmox VE development discussion Cc: pve-devel@lists.proxmox.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Thanks for the review! On Tue Oct 14, 2025 at 5:04 PM CEST, Maximiliano Sandoval wrote: > Christoph Heiss writes: > > Some comments bellow: >> diff --git a/proxinstall b/proxinstall >> index 5ba65fa..35e948a 100755 >> --- a/proxinstall >> +++ b/proxinstall [..] >> +my sub create_network_interface_pin_view { >> + my ($done_cb) = @_; >> + >> + my $dialog = Gtk3::Dialog->new(); >> + $dialog->set_title('Interface Name Pinning Options'); >> + $dialog->add_button('_OK', 1); > > The response argument is indeed a number (gint), but there is an enum > [1] for this. In perl one can use the string 'ok' instead of > GTK_RESPONSE_OK, for example. I'll change it to 'ok', thanks! > > I do not see the value of the response being used during the > `GtkDialog::response` signal handler, note that a dialog can be closed > either be pressing ESC, clicking the X button, or by clicking the `OK` > button as per the callback bellow. As it stands, all the methods I > described above would run the handler equally, is this intended? I tried to be consistent with the advanced disk dialog, which has the exact same behaviour - so yes. [..] >> + >> + $scrolled_window->add($grid); >> + $scrolled_window->set_policy('never', 'automatic'); >> + $scrolled_window->set_visible(1); > > The scrolled window is the child of hbox and gtk_widget_show_all is > called on the later, it should not be necessary to call > gtk_widget_set_visible on this one. I see, I'll remove it. > >> + $scrolled_window->set_min_content_height(200); >> + $scrolled_window->set_margin_end(10); > > It is a bit asymmetrical that there is no margin on the start. Right, thanks for noticing! [..] >> + >> + $dialog->show(); >> + $dialog->run(); > > There are two ways to present dialogs, either by running > `gtk_dialog_run` which will block until the dialog is done and will > return the response (deprecated) and then close/destroy the dialog, or > connect to the response signal which will be emitted once there is a > response and the dialog can be closed (as done above) but instead of > calling `gtk_dialog_run()` one would call `gtk_window_present()` on it. > So please run `present` instead of `run` here. Will do in v2. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel