cli/trivial: add FIXME comment about how wrong editor_init_existing_connection() is

2 files changed, 23 insertions, 0 deletions

diff --git a/clients/cli/connections.c b/clients/cli/connections.c
index 0235de8132..c8a6b37bfa 100644
--- a/clients/cli/connections.c
+++ b/clients/cli/connections.c
@@ -7380,6 +7380,7 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t
 					                          prop_name);
 				}
 
+				/* setting a value in edit mode "appends". That seems unexpected behavior. */
 				if (!nmc_setting_set_property (nmc->client,
 				                               ss,
 				                               prop_name,
@@ -8102,6 +8103,9 @@ editor_init_existing_connection (NMConnection *connection)
 	NMSettingWireless *s_wireless;
 	NMSettingConnection *s_con;
 
+	/* FIXME: this approach of connecting handlers to do something is fundamentally
+	 * flawed. See the comment in nmc_setting_ip6_connect_handlers(). */
+
 	s_ip4 = nm_connection_get_setting_ip4_config (connection);
 	s_ip6 = nm_connection_get_setting_ip6_config (connection);
 	s_proxy = nm_connection_get_setting_proxy (connection);
diff --git a/clients/cli/settings.c b/clients/cli/settings.c
index 7697bfba83..5b13bf03f3 100644
--- a/clients/cli/settings.c
+++ b/clients/cli/settings.c
@@ -142,6 +142,25 @@ ipv6_addresses_changed_cb (GObject *object, GParamSpec *pspec, gpointer user_dat
 		}
 	} else {
 		answered = FALSE;
+		/* FIXME: editor_init_existing_connection() and registering handlers is not the
+		 *  right approach.
+		 *
+		 * This only happens to work because in nmcli's edit mode
+		 * tends to append addresses -- instead of setting them.
+		 * If we would change that (to behavior I'd expect), we'd get:
+		 *
+		 *   nmcli> set ipv6.addresses fc01::1:5/68
+		 *   Do you also want to set 'ipv6.method' to 'manual'? [yes]: y
+		 *   nmcli> set ipv6.addresses fc01::1:6/68
+		 *   Do you also want to set 'ipv6.method' to 'manual'? [yes]:
+		 *
+		 * That's because nmc_setting_set_property() calls set_fcn(). With modifier '\0'
+		 * (set), it would first clear all addresses before adding the address. Thereby
+		 * emitting multiple property changed signals.
+		 *
+		 * That can be avoided by freezing/thawing the signals, but this solution
+		 * here is ugly in general.
+		 */
 		if (!g_strcmp0 (nm_setting_ip_config_get_method (NM_SETTING_IP_CONFIG (object)), NM_SETTING_IP6_CONFIG_METHOD_MANUAL))
 			g_object_set (object, NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_AUTO, NULL);
 	}