GNOME Bugzilla – Bug 766427
[review] lr/connection-add: nmcli c add should allow mixing properties & type options
Last modified: 2016-06-22 08:03:43 UTC
Type options make little sense and their implementation duplicates too much code (validation, conversion from strings). This could be made simpler and less confusing. (Often requested in the survey).
https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?h=lr/connection-add
*** Bug 766748 has been marked as a duplicate of this bug. ***
At a first look seems very good. I pushed some fixups.
Applied the fixups & pushed an updated branch. Added some documentation improvements (new HTML render: https://people.freedesktop.org/~lkundrak/nm-docs/nmcli.html)
there is a static array static OptionInfo option_info[]; Preferably, this static array should be immutable, but enable_options() modifies it. That is, while modifying *one* connection we should not modify global state. Is it possible to pass the mutable part of option_info around?
(In reply to Thomas Haller from comment #5) > there is a static array > static OptionInfo option_info[]; > > > Preferably, this static array should be immutable, but enable_options() > modifies it. That is, while modifying *one* connection we should not modify > global state. > > Is it possible to pass the mutable part of option_info around? Yes. The only reason we mutate the array is to keep state for some obscure logic of the interactive add. Keeping it together has the upside of keeping the all the information needed to determine whether we need to ask for the option in the interactive mode together. I'll prefer to keep it pragmatic at this point and not split it unless really required.
Merged