Re: [evolution-patches] [IMPROVED PATCH]Bug#55208:Warn user about wrong birthdate format in contacts

[Chris Toshok <toshok ximian com>]

On Mon, 2004-04-05 at 08:04 +0530, S N Tejasvi wrote:

> Index: gui/contact-editor/e-contact-editor.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/addressbook/gui/contact-editor/e-contact-editor.c,v
> retrieving revision 1.176
> diff -u -r1.176 e-contact-editor.c
> --- gui/contact-editor/e-contact-editor.c	24 Mar 2004 20:18:46 -0000	1.176
> +++ gui/contact-editor/e-contact-editor.c	6 Apr 2004 14:14:17 -0000
> @@ -1596,14 +1596,14 @@
>  save_contact (EContactEditor *ce, gboolean should_close)
>  {
>  	EditorCloseStruct *ecs = g_new(EditorCloseStruct, 1);
>
>  	extract_info (ce);
>  	if (!ce->target_book)
>  		return;
>  
>  	ecs->ce = ce;
> +	if(!e_contact_editor_is_valid(EAB_EDITOR(ce)))
> +	should_close = FALSE;
>  	g_object_ref (ecs->ce);

you should return from this function if it's !_is_valid.  setting should_close to FALSE will allow the contact to still be saved out, the editor will just remain open.

also move the if up a couple of lines (just below the "if (!ce->target_book)" block).


> @@ -1639,8 +1639,24 @@
>  static gboolean
>  e_contact_editor_is_valid (EABEditor *editor)
>  {
> +
>  	/* insert checks here (date format, for instance, etc.) */
> -	return TRUE;
> +        EContactEditor *ce = E_CONTACT_EDITOR (editor);
> +	GtkWidget *dialog,*widget;
> +	gint result;
> +	widget = glade_xml_get_widget(ce->gui, "dateedit-birthday");
> +
> +	if(!(e_date_edit_date_is_valid(E_DATE_EDIT (widget)))){
> +                        	dialog=gtk_message_dialog_new(GTK_WINDOW(ce->app),
> +                                                               0,
> +                                                               GTK_MESSAGE_INFO,
> +                                                               GTK_BUTTONS_OK,
> +                                                               _("Wrong birth date format"));
> +                                result=gtk_dialog_run(GTK_DIALOG(dialog));
> +                                gtk_widget_destroy(dialog);
> +				return FALSE;
> +         }else
> +                               return TRUE;
>  }

This is mostly ok.  Some minor things - put spaces around all '='s, put spaces before the '(' in a function call, and don't use "}else", put else on the next line, lined up with the first letter under the '}'.  Also, if one branch of an if has {}'s around it, put them around the other as well (the "return TRUE;" should have {}'s around it.)

You should also check both the birth date and anniversary fields.   The error message should probably be built up as a GString and displayed as one message for all the invalid fields.

Something like:

-----
The following fields contain invalid data, and should be fixed:

>     Birth Date
>     Anniversary
-----

and maybe give an example of properly formatted data.  maybe like:

-----
The following fields contain invalid data, and should be fixed:

>     Birth Date
> >         (Examples: 04/04/2004 or Jan-24-1972)
> >     
>     Anniversary
> >         (Examples: 04/04/2004 or Jan-24-1972)
> >     
-----

Of course, someone with more of a brain should determine what the exact text is, but we don't want to be popping up a dialog for every invalid field, and we should give the user some indication of a valid format so they're not just blindly searching.

Chris