Re: [PATCH] editor: change "Device MAC address" to combo box with an entry
- From: Dan Williams <dcbw redhat com>
- To: jklimes redhat com
- Cc: networkmanager-list gnome org
- Subject: Re: [PATCH] editor: change "Device MAC address" to combo box with an entry
- Date: Wed, 20 Jul 2011 23:31:29 -0500
On Tue, 2011-07-19 at 11:01 +0200, Jirka Klimes wrote:
> The attached pached changes "Device MAC address" input entry in wired and
> wireless connections in nm-connection editor to a combo-box with entry.
>
> The combo-box list is pre-filled with available devices' MAC for the connection
> type. Thus it is easier to restrict connections just to a particular interface
> - by selecting a value in the combo. The possibility for inserting others MACs
> is still preserved via manually typing the MAC in the entry.
One suggestion is to make the 'wired' arg to mac_list_data_to_page()
'GType devtype', and pass in either NM_TYPE_DEVICE_ETHERNET or
NM_TYPE_DEVICE_WIFI to it. Then in that function just use G_OBJECT_TYPE
(dev) == devtype and then you don't need the if (wired) and if (!wired)
bits.
But in the end I think we should just pass the NMClient into the CEPage
objects and move mac_list_data_to_page() to ce-page.c if we can. ie add
an "NMClient *client" arg to CEPageNewFunc and let each page call the
generic mac list construction function if it wants to. In the future we
probably want to update the combo dynamically if devices get added or
removed, and we can't do that without having the NMClient in the CEPage
subclass. Plus this makes things a bit cleaner since we don't have to
use a GObject data tag, the function that makes the MAC
address/interface strings can just return a gchar** to the CEPage that
calls it.
In that future with dynamic updating of the combos we'd probably want to
move to using a custom GtkListStore with at least two columns, one
visible one for the interface name/MAC, and an invisible one that has
the MAC as a byte array. A third invisible column could hold the
NMDevice subclass too for tracking add/remove and stuff. But we can do
that later.
The rest looks good.
Thanks!
Dan
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]