[Patch] improve IPv4 setting UI of nm-applet

[cee1 <fykcee1 gmail com>]

Hi Dan:

Attachment contains a series patches of improving IPv4 setting UI of nm-applet.

These patches aim the following problems:

 1) users move focus from a cell of gtktreeview to another widget(i.e. leave gtktreeview) directly, his previous editing will lost.

 2) lack of netmask pre-fill for private IPv4 addresses.

 3) lack of a proper notification of invalid inputs.

Regards

- cee1

From faa34e5a8719df3204fd267cf400f3bc71e22d65 Mon Sep 17 00:00:00 2001
From: cee1 <fykcee1 gmail com>
Date: Sun, 10 Jan 2010 15:19:21 +0800
Subject: [PATCH 1/5] Edit of addr_box should be saved when focus out

The current UI may lost edit of addr_box, when, e.g. user finished
editing addr/prefix/gateway, then moves focus to DNS entry directly,
the edit of gateway will lost.

This patch handle "editing-canceled" signal of each GtkCellRenderer of
addr/prefix/gateway. It should be a better solution if editing state of
treeview more obvious.
---
 src/connection-editor/page-ip4.c |   41 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/src/connection-editor/page-ip4.c b/src/connection-editor/page-ip4.c
index 592d8dd..ea72bfc 100644
--- a/src/connection-editor/page-ip4.c
+++ b/src/connection-editor/page-ip4.c
@@ -93,6 +93,11 @@ typedef struct {
 #define IP4_METHOD_LINK_LOCAL      3
 #define IP4_METHOD_SHARED          4
 
+static struct {
+	GtkWidget *entry;
+	gchar *path;
+} addr_current_input;
+
 static void
 ip4_private_init (CEPageIP4 *self, NMConnection *connection)
 {
@@ -543,6 +548,14 @@ cell_editing_started (GtkCellRenderer *cell,
 		return;
 	}
 
+	/* keep GTK_ENTRY (editable) alive when received signal "editing-canceled" */
+	if (addr_current_input.entry) {
+		g_object_unref (addr_current_input.entry);
+		g_free (addr_current_input.path);
+	}
+	addr_current_input.entry = g_object_ref (editable);
+	addr_current_input.path = g_strdup (path);
+
 	/* Set up the entry filter */
 	g_signal_connect (G_OBJECT (editable), "insert-text",
 	                  (GCallback) ip_address_filter_cb,
@@ -550,6 +563,31 @@ cell_editing_started (GtkCellRenderer *cell,
 }
 
 static void
+cell_editing_canceled (GtkCellRenderer *renderer, gpointer user_data)
+{
+	CEPageIP4 *self = CE_PAGE_IP4 (user_data);
+	CEPageIP4Private *priv = CE_PAGE_IP4_GET_PRIVATE (self);
+	GtkListStore *store = GTK_LIST_STORE (gtk_tree_view_get_model (priv->addr_list));
+	GtkTreePath *path = gtk_tree_path_new_from_string (addr_current_input.path);
+	const gchar *current_text = gtk_entry_get_text (GTK_ENTRY (addr_current_input.entry));
+	GtkTreeIter iter;
+	guint32 column;
+
+	column = GPOINTER_TO_UINT (g_object_get_data (G_OBJECT (renderer), "column"));
+	gtk_tree_model_get_iter (GTK_TREE_MODEL (store), &iter, path);
+	gtk_list_store_set (store, &iter, column, current_text, -1);
+
+	gtk_tree_path_free (path);
+	ce_page_changed (CE_PAGE (self));
+
+	/* reset addr_current_input */
+	g_object_unref (addr_current_input.entry);
+	addr_current_input.entry = NULL;
+	g_free (addr_current_input.path);
+	addr_current_input.path = NULL;
+}
+
+static void
 routes_dialog_close_cb (GtkWidget *dialog, gpointer user_data)
 {
 	gtk_widget_hide (dialog);
@@ -633,6 +671,7 @@ finish_setup (CEPageIP4 *self, gpointer unused, GError *error, gpointer user_dat
 	g_signal_connect (renderer, "edited", G_CALLBACK (cell_edited), self);
 	g_object_set_data (G_OBJECT (renderer), "column", GUINT_TO_POINTER (COL_ADDRESS));
 	g_signal_connect (renderer, "editing-started", G_CALLBACK (cell_editing_started), store);
+	g_signal_connect (renderer, "editing-canceled", G_CALLBACK (cell_editing_canceled), self);
 	priv->addr_cells[COL_ADDRESS] = GTK_CELL_RENDERER (renderer);
 
 	offset = gtk_tree_view_insert_column_with_attributes (priv->addr_list,
@@ -649,6 +688,7 @@ finish_setup (CEPageIP4 *self, gpointer unused, GError *error, gpointer user_dat
 	g_signal_connect (renderer, "edited", G_CALLBACK (cell_edited), self);
 	g_object_set_data (G_OBJECT (renderer), "column", GUINT_TO_POINTER (COL_PREFIX));
 	g_signal_connect (renderer, "editing-started", G_CALLBACK (cell_editing_started), store);
+	g_signal_connect (renderer, "editing-canceled", G_CALLBACK (cell_editing_canceled), self);
 	priv->addr_cells[COL_PREFIX] = GTK_CELL_RENDERER (renderer);
 
 	offset = gtk_tree_view_insert_column_with_attributes (priv->addr_list,
@@ -665,6 +705,7 @@ finish_setup (CEPageIP4 *self, gpointer unused, GError *error, gpointer user_dat
 	g_signal_connect (renderer, "edited", G_CALLBACK (cell_edited), self);
 	g_object_set_data (G_OBJECT (renderer), "column", GUINT_TO_POINTER (COL_GATEWAY));
 	g_signal_connect (renderer, "editing-started", G_CALLBACK (cell_editing_started), store);
+	g_signal_connect (renderer, "editing-canceled", G_CALLBACK (cell_editing_canceled), self);
 	priv->addr_cells[COL_GATEWAY] = GTK_CELL_RENDERER (renderer);
 
 	offset = gtk_tree_view_insert_column_with_attributes (priv->addr_list,
-- 
1.6.5.3

From c902809f140fb611e1c00f1d2b3860ecfc3a4f1d Mon Sep 17 00:00:00 2001
From: cee1 <fykcee1 gmail com>
Date: Sun, 10 Jan 2010 15:42:44 +0800
Subject: [PATCH 2/5] Add netmask prefill for private IPv4 address

---
 src/connection-editor/page-ip4.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/src/connection-editor/page-ip4.c b/src/connection-editor/page-ip4.c
index ea72bfc..1072a26 100644
--- a/src/connection-editor/page-ip4.c
+++ b/src/connection-editor/page-ip4.c
@@ -543,6 +543,9 @@ cell_editing_started (GtkCellRenderer *cell,
                       const gchar     *path,
                       gpointer         data)
 {
+	GtkListStore *store = GTK_LIST_STORE (data);
+	guint32 column;
+
 	if (!GTK_IS_ENTRY (editable)) {
 		g_warning ("%s: Unexpected cell editable type.", __func__);
 		return;
@@ -556,6 +559,38 @@ cell_editing_started (GtkCellRenderer *cell,
 	addr_current_input.entry = g_object_ref (editable);
 	addr_current_input.path = g_strdup (path);
 
+	/* guess DNS for COL_PREFIX */
+	column = GPOINTER_TO_UINT (g_object_get_data (G_OBJECT (cell), "column"));
+	if (column == COL_PREFIX) {
+		gchar *guess_prefixes[] = {
+			"255.0.0.0",    /* A class */
+			"255.255.0.0",  /* B class */
+			"255.255.255.0" /* C class */
+		};
+		const gchar *guess_prefix = NULL;
+		const gchar *prefix = gtk_entry_get_text (GTK_ENTRY (editable));
+
+		gchar *addr;
+		GtkTreeIter iter;
+
+		gtk_tree_model_get_iter_from_string (GTK_TREE_MODEL (store), &iter, path);
+		gtk_tree_model_get (GTK_TREE_MODEL (store), &iter, COL_ADDRESS, &addr, -1);
+
+		if (!prefix || !strlen (prefix)) {
+			if (strncmp ("10.", addr, 3) == 0)
+				guess_prefix = guess_prefixes[0];
+			else if (strncmp ("172.16.", addr, 7) == 0)
+				guess_prefix = guess_prefixes[1];
+			else if (strncmp ("192.168.", addr, 8) == 0)
+				guess_prefix = guess_prefixes[2];
+
+			if (guess_prefix)
+				gtk_entry_set_text (GTK_ENTRY (editable), guess_prefix);
+		}
+
+		g_free (addr);
+	}
+
 	/* Set up the entry filter */
 	g_signal_connect (G_OBJECT (editable), "insert-text",
 	                  (GCallback) ip_address_filter_cb,
-- 
1.6.5.3

From 741aa425ae9fbd29fceff93135d242f0252504ce Mon Sep 17 00:00:00 2001
From: cee1 <fykcee1 gmail com>
Date: Sun, 10 Jan 2010 16:28:27 +0800
Subject: [PATCH 3/5] Add a label for informing of invalid inputs of IPv4

---
 src/connection-editor/ce-page-ip4.glade |   16 ++++++++++-
 src/connection-editor/page-ip4.c        |   43 +++++++++++++++++++++++++++++-
 2 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/src/connection-editor/ce-page-ip4.glade b/src/connection-editor/ce-page-ip4.glade
index 07798fd..d819234 100644
--- a/src/connection-editor/ce-page-ip4.glade
+++ b/src/connection-editor/ce-page-ip4.glade
@@ -52,6 +52,18 @@ Shared to other computers</property>
             <property name="visible">True</property>
             <property name="spacing">6</property>
             <child>
+              <widget class="GtkLabel" id="ip4_addr_info_label">
+                <property name="visible">False</property>
+                <property name="xalign">0.5</property>
+                <property name="use_markup">False</property>
+              </widget>
+              <packing>
+                <property name="expand">False</property>
+                <property name="fill">False</property>
+                <property name="position">0</property>
+              </packing>
+            </child>
+            <child>
               <widget class="GtkLabel" id="ip4_addr_label">
                 <property name="visible">True</property>
                 <property name="xalign">0</property>
@@ -61,7 +73,7 @@ Shared to other computers</property>
               <packing>
                 <property name="expand">False</property>
                 <property name="fill">False</property>
-                <property name="position">0</property>
+                <property name="position">1</property>
               </packing>
             </child>
             <child>
@@ -301,7 +313,7 @@ Shared to other computers</property>
                 </child>
               </widget>
               <packing>
-                <property name="position">1</property>
+                <property name="position">2</property>
               </packing>
             </child>
           </widget>
diff --git a/src/connection-editor/page-ip4.c b/src/connection-editor/page-ip4.c
index 1072a26..bdd0efd 100644
--- a/src/connection-editor/page-ip4.c
+++ b/src/connection-editor/page-ip4.c
@@ -61,6 +61,7 @@ typedef struct {
 
 	/* Addresses */
 	GtkWidget *addr_label;
+	GtkWidget *addr_info_label;
 	GtkButton *addr_add;
 	GtkButton *addr_delete;
 	GtkTreeView *addr_list;
@@ -179,6 +180,7 @@ ip4_private_init (CEPageIP4 *self, NMConnection *connection)
 
 	gtk_combo_box_set_model (priv->method, GTK_TREE_MODEL (priv->method_store));
 
+	priv->addr_info_label = glade_xml_get_widget (xml, "ip4_addr_info_label");
 	priv->addr_label = glade_xml_get_widget (xml, "ip4_addr_label");
 	priv->addr_add = GTK_BUTTON (glade_xml_get_widget (xml, "ip4_addr_add_button"));
 	priv->addr_delete = GTK_BUTTON (glade_xml_get_widget (xml, "ip4_addr_delete_button"));
@@ -301,6 +303,7 @@ populate_ui (CEPageIP4 *self)
 {
 	CEPageIP4Private *priv = CE_PAGE_IP4_GET_PRIVATE (self);
 	NMSettingIP4Config *setting = priv->setting;
+	PangoAttrList *attr_list = NULL;
 	GtkListStore *store;
 	GtkTreeIter model_iter;
 	int method = IP4_METHOD_AUTO;
@@ -328,6 +331,15 @@ populate_ui (CEPageIP4 *self)
 	info.combo = priv->method;
 	gtk_tree_model_foreach (GTK_TREE_MODEL (priv->method_store), set_method, &info);
 
+	/* addr info label: set Error reporting color */
+	attr_list = pango_attr_list_new ();
+
+	/* style: red, 11px, bold */
+	pango_attr_list_insert (attr_list, pango_attr_foreground_new (-1, 0, 0));
+	pango_attr_list_insert (attr_list, pango_attr_size_new (11 * PANGO_SCALE));
+	pango_attr_list_insert (attr_list, pango_attr_weight_new (PANGO_WEIGHT_BOLD));
+	gtk_label_set_attributes (GTK_LABEL (priv->addr_info_label), attr_list);
+
 	/* Addresses */
 	store = gtk_list_store_new (3, G_TYPE_STRING, G_TYPE_STRING, G_TYPE_STRING);
 	for (i = 0; i < nm_setting_ip4_config_get_num_addresses (setting); i++) {
@@ -357,7 +369,6 @@ populate_ui (CEPageIP4 *self)
 	}
 
 	gtk_tree_view_set_model (priv->addr_list, GTK_TREE_MODEL (store));
-	g_signal_connect_swapped (store, "row-inserted", G_CALLBACK (ce_page_changed), self);
 	g_signal_connect_swapped (store, "row-deleted", G_CALLBACK (ce_page_changed), self);
 	g_object_unref (store);
 
@@ -911,37 +922,65 @@ ui_to_setting (CEPageIP4 *self)
 
 		gtk_tree_model_get (model, &tree_iter, COL_ADDRESS, &item, -1);
 		if (!item || !inet_aton (item, &tmp_addr)) {
+			gboolean empty = !item || !strlen (item);
+			gchar *info = empty ? _("Missing IPv4 address") : \
+					g_strdup_printf (_("Invalid IPv4 address '%s'"), item);
+
 			g_warning ("%s: IPv4 address '%s' missing or invalid!",
 			           __func__, item ? item : "<none>");
+			gtk_label_set_text (GTK_LABEL (priv->addr_info_label), info);
+			gtk_widget_show (priv->addr_info_label);
+
+			if (!empty) g_free (info);
 			g_free (item);
+
 			goto out;
 		}
 		g_free (item);
 
 		gtk_tree_model_get (model, &tree_iter, COL_PREFIX, &item, -1);
 		if (!item) {
+			gchar *info = _("Missing IPv4 netmask");
+
 			g_warning ("%s: IPv4 prefix '%s' missing!",
 			           __func__, item ? item : "<none>");
+			gtk_label_set_text (GTK_LABEL (priv->addr_info_label), info);
+			gtk_widget_show (priv->addr_info_label);
+
 			goto out;
 		}
 
 		if (!parse_netmask (item, &prefix)) {
+			gchar *info = g_strdup_printf (_("Invalid IPv4 netmask '%s'"), item);
+
 			g_warning ("%s: IPv4 prefix '%s' invalid!",
 			           __func__, item ? item : "<none>");
+			gtk_label_set_text (GTK_LABEL (priv->addr_info_label), info);
+			gtk_widget_show (priv->addr_info_label);
+			g_free (info);
 			g_free (item);
+
 			goto out;
 		}
 		g_free (item);
 
 		/* Gateway is optional... */
 		gtk_tree_model_get (model, &tree_iter, COL_GATEWAY, &item, -1);
-		if (item && !inet_aton (item, &tmp_gateway)) {
+		if (item && strlen(item) && !inet_aton (item, &tmp_gateway)) {
+			gchar *info = g_strdup_printf (_("Invalid IPv4 gateway '%s'"), item);
+
 			g_warning ("%s: IPv4 gateway '%s' invalid!",
 			           __func__, item ? item : "<none>");
+			gtk_label_set_text (GTK_LABEL (priv->addr_info_label), info);
+			gtk_widget_show (priv->addr_info_label);
+			g_free (info);
 			g_free (item);
 			goto out;
 		}
 		g_free (item);
+		/* clear addr info label */
+		gtk_label_set_text (GTK_LABEL (priv->addr_info_label), NULL);
+		gtk_widget_hide (priv->addr_info_label);
 
 		addr = g_array_sized_new (FALSE, TRUE, sizeof (guint32), 3);
 		g_array_append_val (addr, tmp_addr.s_addr);
-- 
1.6.5.3