Re: [PATCH] editor: change "Device MAC address" to combo box with an entry

[Jirka Klimes <jklimes redhat com>]

On Thursday 21 of July 2011 06:31:29 Dan Williams wrote:
> 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

Thanks for reviewing, Dan.

So I added NMClient object to CEPage and created get_mac_list virtual method.
I stick with GComboBoxText for now as it is simpler.
Pushed to master as a5b87782d833868faa1f5cb67969c3df39e05361.

As far as dynamic updating of the combo is concerned, it's nice to have 
feature and I may look into it later. Though it is probably rare case that 
devices change while the page is open.
Maybe, we should also test if the device MAC is compatible with the connection 
before adding it to the list.

Jirka