Re: Error feedback on graph data selection entries

[Jody Goldberg <jody gnome org>]

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.

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.

> +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

> +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 ?