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 766427 - [review] lr/connection-add: nmcli c add should allow mixing properties & type options
[review] lr/connection-add: nmcli c add should allow mixing properties & type...
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: nmcli
git master
Other Linux
: Normal normal
: ---
Assigned To: Lubomir Rintel
NetworkManager maintainer(s)
: 766748 (view as bug list)
Depends on:
Blocks: nm-review
 
 
Reported: 2016-05-14 10:31 UTC by Lubomir Rintel
Modified: 2016-06-22 08:03 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Lubomir Rintel 2016-05-14 10:31:17 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).
Comment 2 Lubomir Rintel 2016-05-21 12:15:03 UTC
*** Bug 766748 has been marked as a duplicate of this bug. ***
Comment 3 Beniamino Galvani 2016-05-26 14:59:55 UTC
At a first look seems very good. I pushed some fixups.
Comment 4 Lubomir Rintel 2016-05-30 19:10:04 UTC
Applied the fixups & pushed an updated branch.

Added some documentation improvements (new HTML render: https://people.freedesktop.org/~lkundrak/nm-docs/nmcli.html)
Comment 5 Thomas Haller 2016-05-31 17:16:16 UTC
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?
Comment 6 Lubomir Rintel 2016-06-05 07:15:13 UTC
(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.
Comment 7 Lubomir Rintel 2016-06-22 08:03:43 UTC
Merged