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.
Emmanuel.
_______________________________________________
gnumeric-list mailing list
[email protected]
http://mail.gnome.org/mailman/listinfo/gnumeric-list
