Re: Error feedback on graph data selection entries

Le dimanche 16 décembre 2007 à 10:26 -0500, Jody Goldberg a écrit :
The current behavior is broken, and this seems like a reasonable
solution, but the api worries me.  The behaviour should be
consistent across the various prop dialogs.  We're also too close to
1.8.0 to get this properly tested (not your fault).  Please update
with the comments below, and we can try get it in for 1.8.1.

I don't know if we have other open reproductible crasher, but this issue
is almost one.

On Sat, Dec 08, 2007 at 08:51:23PM +0100, Emmanuel Pacaud wrote:
 gnm_expr_entry_parse (GnmExprEntry *gee, GnmParsePos const *pp,
                  GnmParseError *perr, gboolean start_sel,
-                 GnmExprParseFlags flags)
+                 GnmExprParseFlags flags, gboolean update_entry)
This is the only piece that gives me pause.
Why do we need different behaviours for update_entry.

That's where I need your help. I've done the fix in a way it should not
impact other part of gnumeric, by having a parameter that keep the
current behaviour of gnm_expr_entry_parse. I'm a bit puzzled by this
behaviour, and don't understand why a parse function would change the
content of the widget entry. But I didn't remove the entry update
completely, because I don't know how the other part of gnumeric would be
affected by such a change. It seems gnm_expr_entry_parse has always
updated its entry after the parsing. 
+static void
+cb_graph_dim_entry_changed (GnmExprEntry *gee,
+                       GraphDimEditor *editor)
+   if (!GTK_WIDGET_SENSITIVE (gee) || editor->dataset == NULL)
+           return;
+   get_texpr (editor, NULL);
+static void
+cb_graph_dim_editor_update (GnmExprEntry *gee,
+                       G_GNUC_UNUSED gboolean user_requested,
+                       GraphDimEditor *editor)
+   GnmExprTop const *texpr;
+   /* Ignore changes while we are insensitive. useful for displaying
+    * values, without storing then as Data.  Also ignore updates if the
+    * dataset has been cleared via the weakref handler  */
+   if (!GTK_WIDGET_SENSITIVE (gee) || editor->dataset == NULL)
+           return;
+   if (get_texpr (editor, &texpr)) {
That repeated chunk seems like it belongs in get_texpr

I'll change that.

+static gboolean
+get_texpr (GraphDimEditor *editor, GnmExprTop const **new_texpr)
            if (texpr == NULL) {
+                   if (editor->data_type == GOG_DATA_SCALAR) {
+                           if (new_texpr != NULL)
                                    texpr = gnm_expr_top_new_constant (
                                            value_new_string (gnm_expr_entry_get_text (gee)));
Looks like something is missing here.  Don't we want to set is_valid
TRUE if texpr != NULL ?

is_valid is already initialized to TRUE.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]