Re: [evolution-patches] Save-calendar plugin. saving to comma separated files

[Rodrigo Moya <rodrigo novell com>]

On Tue, 2004-11-30 at 11:28 +0100, Philip Van Hoof wrote:
> On Tue, 2004-11-30 at 11:09 +0100, Philip Van Hoof wrote:
> > On Tue, 2004-11-30 at 00:14 +0100, Philip Van Hoof wrote:
> > > Hi there,
> 
> 
> More bugfixes (not attaching here to reduce E-mail traffic)
> 
> 	http://bugzilla.gnome.org/attachment.cgi?id=34319&action=view
> 
> 
it's better, at least IMO, yo also send the patch in the mail. That's
what the list is for.

Some comments on the patch (is this really the last version?)

> +struct _CsvConfig
> +{
struct _CsvConfig {

> +			switch (type)
> +			{
switch (type) {

> +			if (!needquotes) needquotes = string_needsquotes (str, config);
> +			if (str) tmp = g_string_append (tmp, (const gchar*)str);
> +			list = g_slist_next (list); cnt++;
> +			if (list) tmp = g_string_append (tmp, config->delimiter);
> 
again, if (...) should be in its own line

> +	if (!needquotes) 
> +	{
> +		needquotes = strstr (value, config->newline) ? TRUE:FALSE;
> +	}
> +
> +	if (!needquotes) 
> +	{
> +		needquotes = strstr (value, config->quote) ? TRUE:FALSE;
> +	}
the { should be in the same line that the if statement. Also, this code could be better
changed to:

	if (!needquotes) {
		needquotes = strstr (value, config->newline) ? TRUE:FALSE;
		if (!needquotes)
			...
	}

I guess something similar could be done in the following piece of code.

> +static void 
> +ask_destination_and_save (EPlugin *ep, ECalPopupTargetSource *target, ECalSourceType type)
> +{
this function should deal with GTK < 2.4, and using GtkFileChooser/GtkFileSel
accordingly

Apart from that, there are a lot of things to change to match the coding standards,
mainly putting the { char on the same line as the statement, instead of in its own
line.
-- 
Rodrigo Moya <rodrigo novell com>