Re: [gnome-db] New patch. Closes bug #68577

[Rodrigo Moya <rodrigo gnome-db org>]

On Fri, 2002-01-18 at 00:04, Gonzalo Paniagua Javier wrote:
> 	Hi! Here goes again.
> 
> 	Hope this one is ok.
> 
it is almost, but still you've missed some things :-)

> +		typedef long CommandOption;
> +		
>
here, in the IDL, you should define the const values for the options,
and not in the .h files. That is for making them available to anybody
using the IDL.
  
>  
> +const GdaCommandOption GDA_COMMAND_OPTION_IGNORE_ERRORS		= 1;
> +const GdaCommandOption GDA_COMMAND_OPTION_STOP_ON_ERRORS	= 1 << 1;
> +const GdaCommandOption GDA_COMMAND_OPTION_BAD_OPTION		= 1 << 2;
> +
> +const GdaCommandOption GDA_COMMAND_DEFAULT_OPTION = (1 << 1);
> +
all those, move them to the IDL, and provide, as we do with other
IDL-generated types, nice namespaced defines. That is, in gda-command.h:

#define GDA_COMMAND_OPTION_INGORE_ERRORS
GNOME_Database_OPTION_IGNORE_ERRORS
...

>  GdaCommand *
> -gda_command_new (const gchar *text, GdaCommandType type)
> +gda_command_new (const gchar *text, GdaCommandType type, 
> +				    GdaCommandOption option)
>  {
>  	GdaCommand *cmd;
>  
>  	cmd = GNOME_Database_Command__alloc ();
>  	gda_command_set_text (cmd, text);
>  	gda_command_set_command_type (cmd, type);
> +	cmd->option = GDA_COMMAND_OPTION_BAD_OPTION;
> +	gda_command_set_option (cmd, option);
> +	if (cmd->option == GDA_COMMAND_OPTION_BAD_OPTION)
> +		cmd->option = GDA_COMMAND_DEFAULT_OPTION;
>  
also wrong :-) This should check for consistency in the value. That is:

if (cmd->option & GDA_COMMAND_OPTION_IGNORE_ERRORS &&
    cmd->option & GDA_COMMAND_OPTION_STOP_ON_ERRORS)
	cmd->option = GDA_COMMAND_OPTION_DEFAULT;


> +/**
> + * gda_command_get_option
> + * @cmd: a #GdaCommand.
> + *
> + * Gets the command option of @cmd.
> + * 
> + * Returns: the command option of @cmd.
> + */
> +GdaCommandOption
> +gda_command_get_option (GdaCommand *cmd)
> +{
rename it to gda_command_get_options, in plural

> +/**
> + * gda_command_set_option
> + * @cmd: a #GdaCommand
> + * @option: the command option.
> + *
> + * Sets the command option of @cmd. If there conflicting options, it will just
> + * leave the value as before.
> + */
> +void
> +gda_command_set_option (GdaCommand *cmd, GdaCommandOption option)
> +{
the same, gda_command_set_options, in plural

> +	err_mask = GDA_COMMAND_OPTION_IGNORE_ERRORS |
> +		   GDA_COMMAND_OPTION_STOP_ON_ERRORS;
> +
> +	if ((option & err_mask) == err_mask)
> +		return;	// Conflicting options!
> +
this check should be the same as in gda_command_new

> ===================================================================
> RCS file: /cvs/gnome/libgda/libgda/gda-command.h,v
> retrieving revision 1.4
> diff -u -r1.4 gda-command.h
> --- libgda/gda-command.h	2002/01/11 16:37:47	1.4
> +++ libgda/gda-command.h	2002/01/17 22:59:44
> @@ -29,6 +29,14 @@
>  G_BEGIN_DECLS
>  
>  typedef GNOME_Database_Command GdaCommand;
> +
> +typedef GNOME_Database_CommandOption GdaCommandOption;
> +extern const GdaCommandOption GDA_COMMAND_OPTION_IGNORE_ERRORS;
> +extern const GdaCommandOption GDA_COMMAND_OPTION_STOP_ON_ERRORS;
> +extern const GdaCommandOption GDA_COMMAND_OPTION_BAD_OPTION;
> +extern const GdaCommandOption GDA_COMMAND_DEFAULT_OPTION;
> +
replace these ones with the #define's mentioned above.

the rest seems ok if you adapt it to these changes I comment. Sorry to
bother you so many times, but it is quite an important change, and I
don't want to keep anything not taken into account.

cheers
-- 
Rodrigo Moya <rodrigo gnome-db org> - <rodrigo ximian com>
http://www.gnome-db.org/ - http://www.ximian.com/