After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 696940 - keyfile: use sensible names for connection types and sections
keyfile: use sensible names for connection types and sections
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-03-31 00:39 UTC by Pavel Simerda
Modified: 2013-04-12 08:57 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Pavel Simerda 2013-03-31 00:39:23 UTC
A configuration for an Ethernet device currently looks like:

[connection]
id=...
uuid=...
type=802-3-ethernet
[802-3-ethernet]
mac-address=...

Nobody cares about the 802-3 part, even we don't put it in the names of source files which have to be unique. 802-3-ethernet should be changed to just ethernet:

[connection]
id=...
uuid=...
type=ethernet
[ethernet]
mac-address=...
Comment 1 Dan Williams 2013-04-03 13:33:23 UTC
Yeah, we should allow that, though we'll need to do the magic in the keyfile plugin to convert back and forth between 802-3-ethernet <-> ethernet and 802-11-wireless <-> wifi, since the setting names come directly from the keyfile groups and are part of the NM API.
Comment 2 Dan Williams 2013-04-03 16:56:09 UTC
First-pass at this is in dcbw/keyfile-group-names; please review.
Comment 3 Dan Winship 2013-04-08 13:30:33 UTC
messy, but I guess it has to be to support both old and new names

the only thing I wonder is, should the aliases be defined at a lower level (eg, in libnm-util), so that they can be used elsewhere ("nmcli dev") as well?
Comment 4 Dan Williams 2013-04-08 20:37:41 UTC
(In reply to comment #3)
> messy, but I guess it has to be to support both old and new names
> 
> the only thing I wonder is, should the aliases be defined at a lower level (eg,
> in libnm-util), so that they can be used elsewhere ("nmcli dev") as well?

Do you mean the alias functions?  We can't actually change the setting names themselves for backwards compat reasons, but we might be able to make the alias helper functions part of libnm-util.
Comment 5 Jiri Klimes 2013-04-09 09:51:58 UTC
The patch looks good; yeah we have to support both the new aliases (less ugly) and the real setting names (compatibility).

As for nmcli, 'nmcli con show conf <id>' prints real settings name. I'm not sure whether use aliases instead here. This way the names match settings description (html, nm-settings man page).
On the other hand, I define (and accept) aliases for guided connection addition besides the setting names (connection types).
Comment 6 Pavel Simerda 2013-04-09 16:19:45 UTC
Looks nice. I wonder how far should we go with the documentation, nmcli output and stuff like that. But the read/write support is definitely good for me.

What I wonder a little bit about is:

alias = nm_keyfile_plugin_get_setting_for_alias (s);

The variable names imply that we are converting the setting to the alias, while the function name suggests the other direction. It might want clarification.
Comment 7 Dan Williams 2013-04-11 15:58:44 UTC
changed the function names:

nm_keyfile_plugin_get_setting_for_alias -> nm_keyfile_plugin_get_setting_name_for_alias

nm_keyfile_plugin_get_alias_for_setting -> nm_keyfile_plugin_get_alias_for_setting_name

and added some comments for setting_alias_parser() and setting_alias_writer() to clarify.

Pushed, thanks!
Comment 8 Pavel Simerda 2013-04-12 08:57:41 UTC
Thanks. Just a question, shouldn't 'alias' be renamed to 'setting_name' in the following line?

alias = nm_keyfile_plugin_get_setting_name_for_alias (group);

I know this is becoming nitpicking, but at least I will understand your notion of alias, i.e. whether each option is alias to the other or whether you consider the internal setting_name set in stone and have an alias.